Difference between revisions of "Talk:CDT/Archive/Debug/Catchpoints support"

Revision as of 21:52, 16 April 2008

This a page for discussing content of the CDT:_Debug:_Catchpoints_support page. See Wikipedia Talk Page Guidelines for guidelines on editing this page.

Workflow & UI

I think putting the add breakpoint command into the breakpoint view context menu violates some Eclipse UI guidelines. The proper place to place this action would be in the Run top level menu. That said, JDT breaks this rule also by placing the "Add Java Exception Breakpoint" action into the breakpoints view toolbar.

IMO, we should try to define a new retargetable action for "Add exception breakpoint" and collaborate with Java to use it as well.

-Pawel.piech.windriver.com 15:39, 9 April 2008 (EDT)

1. Well I though I use same place where current Add watchpoint is... If we add command to the Run would anybody find it? Actually What guildlines you refer to, I don't think it applies here. Breakpoints view manages breakpoints, user can remove them and user should be able to add them there. It should not be context menu, but it can be toolbar or view menu action.

2. Java exception breakpoint is not the same, it is just of one of Catchpoints variations... If we make it retargetable we have to change its name too

Elaskavaia.qnx.com 15:59, 9 April 2008 (EDT)

1.

• Section 1.7.5.5 "A context menu should be used for context sensitive interaction with the objects in a view." - The "Add Breakpoint..." action is not a selection-oriented action. Granted there are other examples of this, such as "Add Watch Expression..." in the expressions view, but that doesn't make it correct.
• Section 2.3.2 - The whole section is dedicated to why the context menus should be kept short and simple.
• Section 1.7.7.1 "Guideline 9.2: Follow the platform lead when distributing actions within an Action Set." - Platform has established the Run menu as the place to add global breakpoint actions. JDT has also followed this list. I'll admit that the run menu is not the first place I looked for breakpoint actions, but if that is the problem then we should debate this issue with the rest of Eclipse community and ask platform Debug to make changes.
• Putting these commands on the main menu would allow users to disable the corresponding action set when they don't want to see these actions.

2.

• My main point here was that adding debugger-specific breakpoint actions (I mean JDT vs. CDT, vs. etc) makes the whole UI more cluttered and less unusable. This is something that Mikhail also brought and that's why he suggested to make the catchpoint commands be visible only when the corresponding context is active.
• Of course I agree that "Java Exception Breakpoint" is not ideal, maybe "Exception Breakpoint" would do :-) Perhaps I don't understand how catchpoints are semantically different from "Exception" breakpoints. IMO a signal catchpoint could still be though of as a form of an exception breakpoint.

-Pawel.piech.windriver.com 22:48, 9 April 2008 (EDT)

1. Yes I already agreed about context menu, it should not be there. We can add it in Run and in the View. And if we do that we should probably move Add watchpoint in the same place.

2. It is not even Exception breakpoint. It can be stop on loading shared library event.

-Elaskavaia.qnx.com 09:50, 11 April 2008 (EDT)

1. Great I'm glad we agree about the context menu use. However, I'm rather concerned about using the view menu as well because of the UI guidelines: [User_Interface_Guidelines#Menus_2 1.7.5.3]. This is what the Wind River product does and at first it seems like a logical place actions specific to the content of the view, but when you look at other views and what they have in the view menu, none of the standard views have actions which affect the content of the views, only their presentation. As far as the guidelines go, toolbar should also not be used for these actions, but with the java exception BP action there there may be more of a case for precedent. Of course, the problem with the toobar is that it gets cluttered so quickly, we can't afford to add many things there.

2. I think this question depends on the question from the Implementation section: are all catchpoints to be represented by a single action or should there be separate actions for different types of catchpoints.

-Pawel.piech.windriver.com 14:04, 11 April 2008 (EDT)

File -> New

I was thinking a bit about this breakpoint workflow problem over the weekend and I think that this is one pattern in the UI that might be worth imitating or contributing to, in order to manage these specialized breakpoint types. File->New menu and wizard are already used for creating a lot more than files and projects, there's no reason it couldn't be used for creating breakpoints. This sub-menu is created dynamically and is somewhat context-sensitive (based on active perspective I believe). We could try to extend it so that it can also be context sensitive based on the current active debug session.

Also, even though this may go against my earlier comments about the context menu, I think it would make sense to add the "New" sub-menu to the Breakpoints view context menu, just as the Navigator has a "New" in it's context menu.

-Pawel.piech.windriver.com 16:09, 14 April 2008 (EDT)

I think using File > New is very counterintuitive based on precedent not only within Eclipse but in debuggers in general. I really think users are going to scratch their heads if that's where it ends up.

--John.cortell.freescale.com 16:57, 14 April 2008 (EDT)

Currently the only way to add a CDT watchpoint is to right-click in the Breakpoints view and select "Add Watchpoint". Do you think it would be counterintuitive to substitute that with New->Watchpoint ...? File->New is not the first place I would look in to add breakpoints, but that's also true for a lot of other things that are in there, such as CVS repository locations. There is even already a breakpoint reference from the File menu: Import/Export. Finally, it seems that the current place for breakpoint actions, at the bottom of the Run menu is not exactly very popular either :-)

Still I can see why using the File->New mechanism might be a hard sell. Plus on the implementation side, it may be difficult to enable populating of this menu based on the active debug context, that's why I'm also asking about imitating this pattern if it cannot be used out right.

-Pawel.piech.windriver.com 17:25, 14 April 2008 (EDT)

There's actually three views in the Eclipse UI through which you can create a watchpoint, but that's irrelevant for this discussion. On topic, I'm all for a "New >" action in the Breakpoints view to handle all things that end up in the Breakpoints view: breakpoints (regular, hardware, software), watchpoints, catchpoints, and anything else that may show up down the road.

--John.cortell.freescale.com 18:49, 14 April 2008 (EDT)

I think adding "New" to the breakpoints view is a very good idea.

--Mikhail.Khodjaiants.arm.com

I like the idea of New, but it seems better to implement it as an Add button right next to the Remove button. I imagine it bringing up a wizard with an initial page asking the user to select a breakpoint type. An option on the page could be 'Only show breakpoint types supported by selected target'. (Aside: this wizard could also be used to provide a new workflow for attaching actions to a breakpoint.) You could also include a pulldown on the button with dedicated actions that would open directly to the settings of a particular type of breakpoint.

--Tom.hochstein.freescale.com 12:32, 15 April 2008 (EDT)

I looked a little bit at the APIs and extension points used to implement File->New. If this breakpoints were to use this mechanism there are a couple of things to keep in mind:

• The new breakpoint UI would have to be in form of a wizard. This may actually not be such a bad thing, since currently as a user I have no idea what to expect when I select an "Add ... Breakpoint" action, which is somewhat confusing.
• The New action sub-menu is populated based on the perspectiveExtensions/newWizardShortcut extension point contributions. There is no provision for filtering these contributions based on anything but the active perspective. To make this work for breakpoints, I think we would have to extend the newWizardShortcut in with a visibleWhen parameter.

-Pawel.piech.windriver.com 14:05, 15 April 2008 (EDT)

I like idea of using wizard but not in File menu. File->New menu is used to create Resources. Breakpoint is not a resource. From Usability standpoint it is very unnatural and scratched. The reasonable options are:

• Context menu, where Add Watchpoint is right now
• Breakpoint View toolbar/menu (You delete breakpoint there, You add them there)
• Run menu, where other toggle breakpoints actions are

--Elaskavaia.qnx.com 16:08, 15 April 2008 (EDT)

I created bug 227394 with a quick prototype of this proposal. Further discussion on this topic should probably go there.

-Pawel.piech.windriver.com 14:45, 16 April 2008 (EDT)

I like the idea of an extensible wizard for breakpoint creation, I think we could do a lot of interesting things with it. I think it should be opened by an Add button in the Breakpoints View. Lots of people don't find the menus inside views and the button would make it more obvious and fit nicely with the delete buttons.

--Ken.ryall.nokia.com 21:52, 16 April 2008 (EDT)

Catchpoint contributions

This is what BreakpointExtension mechanism would be useful for. -Pawel.piech.windriver.com 15:44, 9 April 2008 (EDT)

Isn't it more on core side? I was thinking more of ui control for specifics arguments. Can you elaborate on that?

-Elaskavaia.qnx.com 16:01, 9 April 2008 (EDT)

There is both a model and a UI side to this. A good example to follow is filtering breakpoints using threads. This page is model specific and there is a separate implementation for DSF. Specific debugger implementations can use this mechanism to contribute breakpoint property pages that are specific to their debugger.

-Pawel.piech.windriver.com 22:54, 9 April 2008 (EDT)

I don't see any ui extensions I can use. I can create extension to org.eclipse.cdt.debug.core.BreakpointExtension

but it does not have any defined interface and I cannot add ui class reference there because it is core...

Threads filter is hardcoded ui, it uses ICBreakpointFilterExtension but user cannot extend it, do I miss something?

-Elaskavaia.qnx.com 13:02, 11 April 2008 (EDT)

Clarification: the breakpoint extension mechanism is useful for adding debugger-specific breakpoint attributes and corresponding UI. By debugger specific I mean GDB, or Wind River, or Frysk, etc. I don't think you would want to use this extension mechanism to implement the generic catchpoint functionality.

To use this mechanism you would need to:

• Implement IBreakpointExtension with a class that declares and stores your debugger-specific attributes,
• Declare the extension using BreakpointExtension point
• Implement and declare a property page to edit properties in the extension (e.g. the CBreakpointFilteringPage)

-Pawel.piech.windriver.com 14:13, 11 April 2008 (EDT)

I looked at that. Unfortunately if I add property page for catchpoint it would look very silly - common page would be empty and misleading. I am thinking to extending Common page to support custom type randering.

-Elaskavaia.qnx.com 15:00, 11 April 2008 (EDT)

After I did some more digging:

• Current extension mechanisms in CDT is not enough to implement new basic type (basic type is type that directly extends CBreakpoint)
• Property Pages is not enough because a) basic information should be displayed in common/main tab b) there is no UI mechanism for creating new breakpoint (using same custom controls as in property page)
• Besides that LabelProvider in Breakpoints view sucks because it would not show label at all for breakpoint which does not belong to one of the basic types.

I suggest first fix these points above:

• Fix label provider, so by default is uses IMarker.MESSAGE attribute. Alternatively/additionally create message providing breakpoint extension interface.
• Add extension point that allow to contribute control that can be used
  • in common property page to show basic information
  • in new breakpoint dialog to allow to create breakpoint specific values

Using stuff above I can implement more concrete CCatchpoint class and default action support for that.

--Elaskavaia.qnx.com 16:44, 11 April 2008 (EDT)

I believe that partly you are running into these problems because you are trying to stretch the existing framework in a way that it was not intended. Breakpoint types (based on markers) are hierarchical and are extensible. It is not without problems, but by inventing a separate mechanism to represent different catchpoint types you are circumventing the problems of the existing framework rather than trying to fix it.

-Pawel.piech.windriver.com 17:12, 11 April 2008 (EDT)

Well we get to the begging again. As I said there is two ways: threat catchpoint event types as different breakpoints in this case we can use general breakpoint extension mechanism. But, besides that it is not supported nicely by CDT, it mean we end up having 10 Add X breakpoint actions from gdb.

Second approach we create specific ICCatchpoint type which manages event types in a way that Watchpoint manages its expression, i.e. as breakpoint attributes. In this case I have to create extension point to manage different breakpoint types and I cannot use breakpoint extension mechanism because I am not extending anything, I am just installing more types.

From your comment I understood that you prefer first approach. But I am not convinced. I don't think there is a point discussing details when we disagree on this major thing.

--Elaskavaia.qnx.com 12:51, 14 April 2008 (EDT)

Agreed. I hope we can get some more points of view in this discussion.

-Pawel.piech.windriver.com 16:00, 14 April 2008 (EDT)

Implementation

I think the implication here is that there would be a single action to create an event for any type of catchpoint. I'm a little concerned that this may be cramming too much unneeded complexity in the UI and in the API layers, because the needs of different types of catchpoints may be rather different. I vote for using different breakpoint types for different types of catchpoints, although this would make it all the more important to be able to filter the actions for these breakpoints based on debugger capabilities.
-Pawel.piech.windriver.com 23:03, 9 April 2008 (EDT)

I think gcc provides 10-12 different types of catchpoints. Adding them as separate action would be a way to much.

Don't you think?

-Elaskavaia.qnx.com 09:45, 11 April 2008 (EDT)

If these actions are not filtered in any way, then yes I think it would be too much. But as was indicated by Mikhail, the existing actions such as "Add Watchpoint" are already creating too much clutter for debuggers that don't implement them. So IMO it's a requirement for this feature to implement filtering of these actions based on the capabilities of the debugger. If the existing actions filtering features such as action sets, capabilities, command expressions, are not sufficient to implement appropriate filtering for this feature, then we should work with platform to improve them. I realize that this greatly complicates this feature, but it's really important to the overall usability of the CDT debugger.

Basically there are two options:

Option 1) I can add catchpoint specific breakpont type ICCatpoint, it will be similar to other types defined previously, such as ICWatpoint, ICAddressBreakpointe etc. In this case we use schema meaning marker type = catchpoint, specific types are marker attributes. I modify all places that has right now hardcoded switch for breakpoint interface type. This automatically mean it has only one action to add catchpoint.

Option 2) Instead we can add support for generic breakpoint extensions, there is some code there but there is no full support for that. Meaning in all "switches" we add proper handling of default interface (ICBreakpoint). Also it mean we add CGenericBreakpoint class because CBreakpoint itself is abstract class. In this case separate type can be created for each catchpoint event. The only problem I have with that is it will UI very cluttered.

-Elaskavaia.qnx.com 12:44, 11 April 2008 (EDT)

I also see Option 3) Add specific breakpoint types for different types of catchpoints: ICExceptionBreakpoint, ICSignalBreakpoint, ICThreadEventBreakpoint, ICProcessEventBreakpoint, ICLibraryEventBreakpoint, etc.

-Pawel.piech.windriver.com

That would not be too good considering amount of hardcoded subtype processing in curernt code. Extensions can cover this nicely.

--Elaskavaia.qnx.com 15:04, 11 April 2008 (EDT)

The effort you'll spend implementing a new extension mechanism could just as well be directed towards treating breakpoint types more generically in CDT. Once again I think this comes down to the question of whether to fix the existing framework or to work around it.

-Pawel.piech.windriver.com 17:13, 11 April 2008 (EDT)