Bug 474179 - [breakpoints] Adding a breakpoint with "Add Line Breakpoint (C/C++)" is not reflected in vertical ruler
Summary: [breakpoints] Adding a breakpoint with "Add Line Breakpoint (C/C++)" is not r...
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-debug (show other bugs)
Version: 8.8.0   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 8.8.0   Edit
Assignee: Jonah Graham CLA
QA Contact: Doug Schaefer CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-03 15:27 EDT by Marc-André Laperle CLA
Modified: 2015-09-09 09:33 EDT (History)
4 users (show)

See Also:


Attachments
error on function breakpoint (16.81 KB, image/png)
2015-08-17 14:18 EDT, Tracy Miranda CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Marc-André Laperle CLA 2015-08-03 15:27:24 EDT
Using CDT 8.8.0.201508011003

1. Create a Hello world C project
2. Start debugging
3. In the Breakpoints view, use Add Line Breakpoint to add a breakpoint inside main (main.c line 16).

The breakpoint marker does not appear in the vertical ruler of the editor.
Comment 1 Jonah Graham CLA 2015-08-07 11:37:06 EDT
What is going on here is that the user in this case has inserted the breakpoint on main.c:16, which despite appearances is different that /path/to/main.c:16. The former's resource is the workspace root, the latter is the main.c in the project.

With full paths on in the breakpoint table, it looks like this:
main.c [line: 16]	
/scratch/eclipse/src/cdt/runtime-CDT/HelloWorld/src/main.c [line: 15]	

Until a debug session is running the former (main.c:16) has no resolution to resources. 

Therefore, I think the full solution to this problem is to create installed breakpoint markers for the resolved breakpoints.

There are other cases that have the same problem. For example, doing a "b main.c:16" when that creates a pending breakpoint causes the platform breakpoint to be created without a full path, but do the same "b main.c:16" after that file has been loaded (ie not pending breakpoint) causes the platform breakpoint to be created with the resolved path. 

I know some work was done on trying to show relocated breakpoints (similar, problem) and perhaps that can be used here too. 


Perhaps in the meantime of resolving the above inconsistencies I could do something to ameliorate the users experience. I am not not keen on requiring the file to exist to enable the insertion, but perhaps a warning would be appropriate? I will submit a patch for review to see if this meets expectations.
Comment 2 Eclipse Genie CLA 2015-08-07 11:37:59 EDT
New Gerrit change created: https://git.eclipse.org/r/53419
Comment 3 Marc Khouzam CLA 2015-08-10 15:20:39 EDT
(In reply to Jonah Graham from comment #1)

> There are other cases that have the same problem. For example, doing a "b
> main.c:16" when that creates a pending breakpoint causes the platform
> breakpoint to be created without a full path, but do the same "b main.c:16"
> after that file has been loaded (ie not pending breakpoint) causes the
> platform breakpoint to be created with the resolved path. 

Interesting.
We should probably update the platform breakpoint once the debugger resolves it.  But that is not the goal of this bug.  We should have another bug for this particular situation.

> I know some work was done on trying to show relocated breakpoints (similar,
> problem) and perhaps that can be used here too. 
>  
> Perhaps in the meantime of resolving the above inconsistencies I could do
> something to ameliorate the users experience. I am not not keen on requiring
> the file to exist to enable the insertion, but perhaps a warning would be
> appropriate? I will submit a patch for review to see if this meets
> expectations.

I tried the warning patch and to be honest, I didn't think it helped much.

Would it be a valuable enough step to update the platform breakpoint to the full file path based on the debugger ^done response?  So, when the bp is created using "Add Line Breakpoint (C/C++)", we could check if we have an absolute path, and if not, we could use the debugger response to set that path.
Comment 4 Jonah Graham CLA 2015-08-16 11:28:17 EDT
I have submitted an updated gerrit for making this case an error. That should resolve this bug and the other issues (i.e. displaying installed breakpoints) are not for the 8.8 release.
Comment 6 Marc-André Laperle CLA 2015-08-17 11:09:12 EDT
(In reply to Eclipse Genie from comment #5)
> Gerrit change https://git.eclipse.org/r/53419 was merged to [master].
> Commit:
> http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/
> ?id=bd6fa94e63387dbaf42ce0f5f426cf1f1b1b4d00

Thanks, I tested the patch and it's much clearer. One small thing though. Normally, Eclipse dialogs do not display errors until the user has entered information. See:
https://eclipse.org/articles/Article-UI-Guidelines/Contents.html#Wizards

"When a wizard first opens, the focus should be placed in the first field requiring information (see Guidelines 3.1).  The header should be used to prompt the user for the first piece of required information. 

...

It is not appropriate to display an error message.  At this point, the user hasn't done anything yet."
Comment 7 Eclipse Genie CLA 2015-08-17 11:29:01 EDT
New Gerrit change created: https://git.eclipse.org/r/53908
Comment 9 Marc Khouzam CLA 2015-08-17 11:30:44 EDT
(In reply to Marc-Andre Laperle from comment #6)

I've committed Jonah improvement to master and cdt_8_8 so as to have it for RC1.
 
> "When a wizard first opens, the focus should be placed in the first field
> requiring information (see Guidelines 3.1).  The header should be used to
> prompt the user for the first piece of required information. 
> 
> ...
> 
> It is not appropriate to display an error message.  At this point, the user
> hasn't done anything yet."

This is a worthwhile improvement that can go in for RC2.  Jonah, are you able to have a look at that?
Comment 10 Tracy Miranda CLA 2015-08-17 14:17:50 EDT
(In reply to Marc-Andre Laperle from comment #6)
> (In reply to Eclipse Genie from comment #5)
> > Gerrit change https://git.eclipse.org/r/53419 was merged to [master].
> > Commit:
> > http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/
> > ?id=bd6fa94e63387dbaf42ce0f5f426cf1f1b1b4d00
> 
> Thanks, I tested the patch and it's much clearer. One small thing though.
> Normally, Eclipse dialogs do not display errors until the user has entered
> information. See:
> https://eclipse.org/articles/Article-UI-Guidelines/Contents.html#Wizards
> 
> "When a wizard first opens, the focus should be placed in the first field
> requiring information (see Guidelines 3.1).  The header should be used to
> prompt the user for the first piece of required information. 
> 
> ...
> 
> It is not appropriate to display an error message.  At this point, the user
> hasn't done anything yet."

I agree completely, but that is indeed a bug for another day as it requires at least some amount of refactoring of all the insert breakpoint dialogs (and the underlying property pages they use). I will next attach a screenshot of the add function breakpoint for comparison.
Comment 11 Tracy Miranda CLA 2015-08-17 14:18:28 EDT
Created attachment 255904 [details]
error on function breakpoint
Comment 12 Marc-André Laperle CLA 2015-08-17 14:34:54 EDT
(In reply to Tracy Miranda from comment #10)
> I agree completely, but that is indeed a bug for another day as it requires
> at least some amount of refactoring of all the insert breakpoint dialogs
> (and the underlying property pages they use). I will next attach a
> screenshot of the add function breakpoint for comparison.

Sounds good, it can be addressed in a separate bug/patch. It's pretty minor.
Comment 13 Marc Khouzam CLA 2015-09-09 09:33:49 EDT
I believe the reason for this bug has been addressed properly