Community
Participate
Working Groups
I have a use case in CDT where I would like to open a dialog and edit breakpoint settings before creating the breakpoint. I would like to use the Ctrl key as a modifier to the toggle breakpoint action to trigger the dialog to open. To implement this I'd like to add an IToggleBreakpointAdapterExtension2 with takes a toggleBreakpointsWithEvent() method. Then extend the toggle breakpoint actions to use this new interface.
Good timing on this feature. The blocking bug in text editor was just fixed in 3.8M5.
Created attachment 210383 [details] Patch woth proposed changes. My original idea of adding the modifier to the toggle API wasn't such a good idea: the various ways to invoke toggle breakpoint action can have different modifier keys. Instead I added "toggleBreakpointsInteractive" method. I think this is more explicit and thus clear. The implementation of toggleBreakpointsInteractive can determine itself what action to take but the guideline is: - if a breakpoint does not exist at given location, client should open a dialog or wizard to let user edit a breakpoint and then create it. - if a breakpoint already exists at given location, debugger should open a dialog to edit the breakpoint properties. I'll attach the jdt implementation to use this API separately to bug Bug 346615 .
I'm still working on the CDT implementation, but I don't think it'll impact these proposed API changes. Mike could you review this patch when you have some time?
Looking beyond the double click use case I think it'll be good to make the api even more specific: For CDT, I'm looking at adding a ruler context menu action "Add Breakpoint...". However, if I only have toggleBreakpointInteractive() to call, I cannot tell for sure whether the action will add a new breakpoint or edit an existing breakpoint. I think I'll have to replace toggleBreakpointInteractive() with: canOpenBreakpointProperties(), openBreakpointProperties(), canAddBreakpointInteractive(), addBreakpointInteractive(). I'll refactor the patches and post them again.
(In reply to comment #3) > I'm still working on the CDT implementation, but I don't think it'll impact > these proposed API changes. Mike could you review this patch when you have > some time? Some immediate thoughts: 1. perhaps rename method to toggleBreakpointsWithEvent and add in the event (akin to runWithEvent) 2. we need some way for the user to change what the modifiers do, I'm not sure yet if this should be a debug-specific thing or not. > I'll refactor the patches and post them again. I think if we pass in the complete event (with the key mask details) then clients can figure out what they want to do, and we won't have to add a pile more methods.
(In reply to comment #5) > I think if we pass in the complete event (with the key mask details) then > clients can figure out what they want to do, and we won't have to add a pile > more methods. That was my original intent. However, I then realised that the toggle target is most often invoked by the ruler double click as well as by Ctrl-Shift-B accelerator. In the latter case the modifier flags would always be passed to the toggle target (I didn't actually test this assumption, but I think it's the case). We could get around this problem by overriding the modifier mask for the key-accelerator case, but then we'd really be turning the event mask into kind of a poorly named general purpose parameter meant to alter the toggle behaviour. I don't really care to invent more interfaces (we have plenty already ;-)), but it seems like the most correct thing to do. After working through some use cases yesterday, I'm actually starting to lean more closely to your original patch: - Create a new adapter interface dedicated to editing the breakpoint properties IEditBreakpointsTarget. This interface could be retrieved from the breakpoint as an adapter and we can provide a default one for IBreakpoint. - Make the IToggle...2 interface deal specifically with creating the breakpoint interactively, and not overload it with the edit breakpoint properties functionality. Then JDT wouldn't even need to implement this one at all. What do you think?
Created attachment 210532 [details] Updated patch. I rewrite the patch to separate the two interfaces for editing breakpoint properties and adding a breakpoint interactively. I also added an action delegate for a "Add Breakpoint..." that debuggers can contribute to an editor. A breakpoint properties action delegate is not needed as an API since, unlike the former, it does not need access to the internal ToggleBreakpointsTargetManager class.
Created attachment 210834 [details] Updated reduced patch. This version of the patch takes a different approach to open up the API: instead of adding another extension to the toggle breakpoint target, it exposes the IToggleBreakpointManager as an API. The toggle breakpoint manager is a utility for tracking the registered toggle adapters. Making it an API allows 3rd parties to implement specialised toggle breakpoint actions without needing them to be in platform debug project. I'll post the CDT patch using this API in the separate bug.
I've validated the API with the CDT feature I want to implement (bug 360588) and I think making the toggle manager API is much more powerful than creating the individual actions as needed. I'd like to commit this patch soon, so I can pick up the changes in CDT.
(In reply to comment #9) > I've validated the API with the CDT feature I want to implement (bug 360588) > and I think making the toggle manager API is much more powerful than creating > the individual actions as needed. I'd like to commit this patch soon, so I can > pick up the changes in CDT. -1 for the reduced patch. I confirmed that using the keybinding (Ctrl+Shift+B) does not call runWithEvent, so I think the patch can be made even simpler yet. I think we should stick to making it as absolutely minimal as possible. I will try to code up an example for the platform tomorrow.
(In reply to comment #10) > I think we should stick to making it as absolutely minimal as possible. Do you mean you'd rather not expose the IToggleBreakpointsTargetManager as an API, or that you want to make the actions more compact. I started to count on having the access to toggle manager to implement more exotic breakpoint actions in CDT. For one example, I'd like to give CDT clients ability to provide a toggle targets for CDT's "event" breakpoints, and I need a new toggle action for that purpose.
(In reply to comment #11) > (In reply to comment #10) > > I think we should stick to making it as absolutely minimal as possible. > > Do you mean you'd rather not expose the IToggleBreakpointsTargetManager as an > API, or that you want to make the actions more compact. I started to count on > having the access to toggle manager to implement more exotic breakpoint actions > in CDT. For one example, I'd like to give CDT clients ability to provide a > toggle targets for CDT's "event" breakpoints, and I need a new toggle action > for that purpose. I attached a platform patch to bug 346615 that shows what I am referring to.
(In reply to comment #12) > I attached a platform patch to bug 346615 that shows what I am referring to. Thank you Mike, I think now I understand your main concern: you want to be able to interpret the event flags inside the toggle adapter and let that toggle adapter interpret those flags depending on the needs of that debugger. This make sense for CDT too, although I have an additional use cases that require additional API beyond the toggle event flags. For example I would like to have an action which acts on the active toggle target, but instead of toggling the breakpoint on/off, it should always create the breakpoint (interactively with a dialog). I would also like to use the toggle framework to add other types of breakpoints not currently spelled out by the toggle target (event breakpoints, register breakpoints, etc.). So I think the best thing to do is to separate bug 346615 from this one. You already posted the patch for bug 346615, I'll post a reduced patch in this bug that only adds the IToggleBreakpointsTarget manager as an API.
Created attachment 211086 [details] IToggleBreakpointsTargetManager ony patch This patch only adds the toggle manager to the API for clients to use to create custom toggle actions.
Mike, do you see any issues left with adding this API for 3.8?
Created attachment 211198 [details] Rebased patch on top of master (including patch from bug 346615). I also removed IToggleBreakpointTargetManager.getBreakpointFromEditor() and added a DebugUITools.getDebugContextForPart() convenience method.
Is it possible to create custom doubleclick action for non-breakpoint markers? Our IDE has some other markers on the ruler. Currently it will toggle breakpoint if the user double-clicks our marker. It would be nice if we could change this behavior for our markers.
+1 for the reduced patch.
(In reply to comment #18) > +1 for the reduced patch. Thanks! I pushed the fix to origin: http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?id=aa357e78b41e87b7d414d1a7d0b8b7db0dfce011 (In reply to comment #17) > Is it possible to create custom doubleclick action for non-breakpoint markers? > > Our IDE has some other markers on the ruler. Currently it will toggle > breakpoint if the user double-clicks our marker. It would be nice if we could > change this behavior for our markers. It depends I guess. If your IDE uses a custom editor and you control the toggle action that is registered for the double-click action. Then you can implement your own toggle action which recognises these other markers in addition to the breakpoint markers. This new API allows you to implement this custom toggle without accessing the internal ToggleBreakpointsTargetManager class. However if you contribute extensions to another project, such as CDT or JDT, you'll need to ask that project to update its toggle breakpoint action so that you can extend it.
