Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 315443

Summary: [disassembly] Not possible to remove breakpoint in disassembly view with double clicks
Product: [Tools] CDT Reporter: Patrick Chuong <pchuong>
Component: cdt-debugAssignee: Patrick Chuong <pchuong>
Status: RESOLVED FIXED QA Contact: Ken Ryall <ken.ryall>
Severity: normal    
Priority: P3 CC: aleherb+eclipse, kirk.beitz, pawel.1.piech
Version: 7.0Flags: pchuong: iplog-
Target Milestone: 8.0   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Bug Depends on: 333946    
Bug Blocks:    
Attachments:
Description Flags
Patch for disassembly selection pchuong: iplog-

Description Patrick Chuong CLA 2010-06-02 14:18:46 EDT 
Build Identifier: 201005241228

You can set a breakpoint by double clicks on the label line in the disassembly view, but it is not possible to remove the breakpoint by double clicks on the same line. To remove the breakpoint, you need to move to the next line where the breakpoint is shifted. Ideally, the breakpoint should be set at the label line.

You can double clicks on the source line to set a breakpoint, and the breakpoint is not shifted. Label should have the same behavior as source line breakpoint. 

Reproducible: Always
Comment 1 Patrick Chuong CLA 2010-12-17 17:46:47 EST 
Created attachment 185466 [details]
Patch for disassembly selection

Anton, can you please review this patch? I added one additional API to the IDisassemblySelection, this will enable debugger to retrives label from the selection and plant label (symbolic) breakpoint.
Comment 2 Anton Leherbauer CLA 2010-12-20 04:50:08 EST 
(In reply to comment #1)
> Anton, can you please review this patch? I added one additional API to the
> IDisassemblySelection, this will enable debugger to retrives label from the
> selection and plant label (symbolic) breakpoint.

Seems fine, except for a few minor issues:

- unused constant CATEGORY_LABELS in DisassemblySelection
- missing @since tag for IDisassemblySelection.getLabel() and its implementation
Comment 3 Patrick Chuong CLA 2010-12-20 10:38:01 EST 
Reviewed by Anton and made the suggested fix and committed to HEAD.
Comment 5 Kirk Beitz CLA 2010-12-23 20:22:26 EST 
this looks like it could be useful to us in 7.0.2 , which our next delivery is relying on.

would it be possible to get this patch committed to CDT_7_0 as well?  the diff makes it appear that it should be straightforward to do so.
Comment 6 Patrick Chuong CLA 2011-01-04 11:38:46 EST 
I think it is safe to port it back to 7.0.2. 

Anton, do you have any concern about Kirk's request?
Comment 7 Kirk Beitz CLA 2011-01-04 20:28:43 EST 
(In reply to comment #6)
> I think it is safe to port it back to 7.0.2. 
> 
> Anton, do you have any concern about Kirk's request?

we would need this pretty quickly if it's going to happen.  we have a delivery coming up very soon.
Comment 8 Patrick Chuong CLA 2011-01-05 10:11:19 EST 
I can do the merge. There are few things that I am not sure and would need clarification from some one that has experience.

1. 7_0 branch is the next 7.x release, right?

2. Is new API allow for 7_0 branch (CDT 7.0.2)? This is an internal provisional API.

3. I'll need to increment the minor version for org.eclipse.cdt.dsf.ui, what will the version be?
Comment 9 Anton Leherbauer CLA 2011-01-10 03:49:00 EST 
(In reply to comment #8)

Sorry for the delay, I was out on vacation.
I am OK with backporting this fix to 7.0.2.

> I can do the merge. There are few things that I am not sure and would need
> clarification from some one that has experience.
> 
> 1. 7_0 branch is the next 7.x release, right?

Branch cdt_7_0 is for the next 7.0.x release, ie. 7.0.2.

> 2. Is new API allow for 7_0 branch (CDT 7.0.2)? This is an internal provisional
> API.

IDisassemblySelection is marked as @noimplement and the change is backwards compatible, so I don't see a problem with it.

> 3. I'll need to increment the minor version for org.eclipse.cdt.dsf.ui, what
> will the version be?

You don't need to increment the minor version and the micro version was already incremented for 7.0.2.
Comment 10 Patrick Chuong CLA 2011-01-10 10:16:18 EST 
(In reply to comment #9)

Hi Anton,

> You don't need to increment the minor version and the micro version was 
> already incremented for 7.0.2.

I am referring to org.eclipse.cdt.dsf.ui plugin's manifest file, tt is giving me an error for the new API. Should the manifest file be incremented? Perhaps I am doing something wrong, I have my API tooling pointed to CDT7.0 master and have @since 2.1 tag for the new API in IDisassemblySelection.
Comment 11 Anton Leherbauer CLA 2011-01-11 02:56:06 EST 
(In reply to comment #10)
> I am referring to org.eclipse.cdt.dsf.ui plugin's manifest file, tt is giving
> me an error for the new API. Should the manifest file be incremented? Perhaps I
> am doing something wrong, I have my API tooling pointed to CDT7.0 master and
> have @since 2.1 tag for the new API in IDisassemblySelection.

I see.  The internal.provisional package is marked as public API.  It should be internal, though.  Correcting this creates an API error as well, but I think it is the proper thing to do.  I have created bug 333946 for that.
Comment 12 Patrick Chuong CLA 2011-01-11 10:04:11 EST 
I have committed the patch back to the 7_0 branch.