Community
Participate
Working Groups
To fully integrate the CDT debugger into a another IDE/editor, some CDT UI internal APIs have to be used. It would be good to modify CDT (perhaps exposing some more APIs publicly) so the integration could be done without using internal APIs. For reference, these where the internal APIs used: This whole class is instantiated in the code of custom TabGroup: org.eclipse.cdt.dsf.gdb.internal.ui.launching.LocalApplicationCDebuggerTab These action classes are referred to in the plugin.xml, to add them to a custom editor: org.eclipse.cdt.debug.internal.ui.actions.breakpoints.CBreakpointPropertiesRulerActionDelegate org.eclipse.cdt.debug.internal.ui.actions.breakpoints.CAddBreakpointInteractiveRulerActionDelegate org.eclipse.cdt.debug.internal.ui.actions.AddExpressionEditorActionDelegate org.eclipse.cdt.debug.internal.ui.actions.ResumeAtLineActionDelegate org.eclipse.cdt.debug.internal.ui.actions.MoveToLineActionDelegate Also, related to the actions contributed above, this adapter factory contribution was also added, which uses some more CDT internal API. <factory class="org.eclipse.cdt.debug.internal.ui.actions.RetargettableActionAdapterFactory" adaptableType="com.googlecode.goclipse.editors.GoEditor"> <adapter type="org.eclipse.debug.ui.actions.IRunToLineTarget"/> <adapter type="org.eclipse.cdt.debug.internal.ui.actions.IResumeAtLineTarget"/> <adapter type="org.eclipse.cdt.debug.internal.ui.actions.IMoveToLineTarget"/> </factory>
> However, to fully integrate the CDT debugger into a custom language/editor, > some CDT UI internal APIs had to be used, and that worries me a bit. It > would be good to modify CDT (perhaps exposing some more APIs publicly) so > the integration could be done without using internal APIs. Many classes were kept internal because they were not needed by anyone (or at least no one spoke up). If you need to use them, many can be made public. > For reference, > these where the internal APIs used: > > This whole class is instantiated in the code of custom TabGroup: > org.eclipse.cdt.dsf.gdb.internal.ui.launching.LocalApplicationCDebuggerTab There is a new UI planned for launching and much talk about changing lauches altogether. We may not want to make these public just yet, until we have settled on an approach to launching. > These action classes are referred to in the plugin.xml, to add them to a > custom editor: > org.eclipse.cdt.debug.internal.ui.actions.breakpoints. > CBreakpointPropertiesRulerActionDelegate > org.eclipse.cdt.debug.internal.ui.actions.breakpoints. > CAddBreakpointInteractiveRulerActionDelegate > org.eclipse.cdt.debug.internal.ui.actions.AddExpressionEditorActionDelegate > org.eclipse.cdt.debug.internal.ui.actions.ResumeAtLineActionDelegate > org.eclipse.cdt.debug.internal.ui.actions.MoveToLineActionDelegate I didn't look in detail, but those should not be a problem to make public. > Also, related to the actions contributed above, this adapter factory > contribution was also added, which uses some more CDT internal API. > <factory > > class="org.eclipse.cdt.debug.internal.ui.actions. > RetargettableActionAdapterFactory" > adaptableType="com.googlecode.goclipse.editors.GoEditor"> > <adapter type="org.eclipse.debug.ui.actions.IRunToLineTarget"/> > <adapter > type="org.eclipse.cdt.debug.internal.ui.actions.IResumeAtLineTarget"/> > <adapter > type="org.eclipse.cdt.debug.internal.ui.actions.IMoveToLineTarget"/> > </factory> Those above should not be a problem either. I would like to get other people's opinions, but maybe we can keep those internal classes and make them extend the new public classes. That would avoid breaking anyone that uses those internal classes. Patches welcomed.
(In reply to Marc Khouzam from comment #1) > > These action classes are referred to in the plugin.xml, to add them to a > > custom editor: > > org.eclipse.cdt.debug.internal.ui.actions.breakpoints. > > CBreakpointPropertiesRulerActionDelegate > > org.eclipse.cdt.debug.internal.ui.actions.breakpoints. > > CAddBreakpointInteractiveRulerActionDelegate > > org.eclipse.cdt.debug.internal.ui.actions.AddExpressionEditorActionDelegate > > org.eclipse.cdt.debug.internal.ui.actions.ResumeAtLineActionDelegate > > org.eclipse.cdt.debug.internal.ui.actions.MoveToLineActionDelegate > > I didn't look in detail, but those should not be a problem to make public. > Sounds good. But also, it might be a good opportunity to convert these action contributions to the Platform Command framework, instead of using the deprecated viewerContribution extensions. > > Also, related to the actions contributed above, this adapter factory > > contribution was also added, which uses some more CDT internal API. > > <factory > > > > class="org.eclipse.cdt.debug.internal.ui.actions. > > RetargettableActionAdapterFactory" > > adaptableType="com.googlecode.goclipse.editors.GoEditor"> > > <adapter type="org.eclipse.debug.ui.actions.IRunToLineTarget"/> > > <adapter > > type="org.eclipse.cdt.debug.internal.ui.actions.IResumeAtLineTarget"/> > > <adapter > > type="org.eclipse.cdt.debug.internal.ui.actions.IMoveToLineTarget"/> > > </factory> > > Those above should not be a problem either. > Cool. Although it sounds like the above code could be refactored somehow? It seems redundant to have to specify that adapter factory in addition to the Action delegate. The RetargettableActionAdapterFactory doesn't even use the adaptableObject at all. But I haven't looked into the details. > I would like to get other people's opinions, but maybe we can keep those > internal classes and make them extend the new public classes. That would > avoid breaking anyone that uses those internal classes. > > Patches welcomed. I will keep an eye on this, and consider submitting a patch. But at the moment this is of low priority (and not trival to patch for a CDT newcomer). Also even CDT debugger integration wise there are some more pressing issues.
I've finally got up to writing a patch to address this issue, the first iteration of it being here: https://git.eclipse.org/r/#/c/30274/ There are basically two areas to look at: A) making internal API public. B) changing some Debug UI code to add more extension/override points to allow further UI customization by third-party IDEs. On my patch I focused only on point B, for the moment, since it's far more important. Without this patch, to achieve the goals of B, it was necessary to duplicate a certain amount of sensitive CDT Debug code in the third-party IDE (obviously bad since it makes things brittle). And a few use cases were not possible at all. Looking forward to feedback to getting this patch (and subsequent ones) accepted.
I know Marc mentioned that we didn't make them API because no one wanted them, but I actually think that's the wrong approach which has ended up with people using the internal APIs even though we didn't support them. GDB is a universal debugger, we should provide APIs that allow other IDEs for all the languages it supports to reuse the hard work we've done and enable them to come help. </motherhood statement> :)
I'll have a look.
(In reply to Bruno Medeiros from comment #3) > I've finally got up to writing a patch to address this issue, the first > iteration of it being here: > https://git.eclipse.org/r/#/c/30274/ > > There are basically two areas to look at: > A) making internal API public. > B) changing some Debug UI code to add more extension/override points to > allow further UI customization by third-party IDEs. > > On my patch I focused only on point B, for the moment, since it's far more > important. Without this patch, to achieve the goals of B, it was necessary > to duplicate a certain amount of sensitive CDT Debug code in the third-party > IDE (obviously bad since it makes things brittle). And a few use cases were > not possible at all. > > Looking forward to feedback to getting this patch (and subsequent ones) > accepted. I like the changes and I'd like to see them get in. However, I don't know how to handle some of the API impacts. The patch makes some classes API, but those classes extend other classes that are not API. There are a bunch of warnings about that. What are we supposed to do in this case? 1- Do we try to make the base classes API? Those are in DSF and we could make them API. 2- Or do we ignore the warnings and just make the DSF-GDB classes API? I'm thinking that #2 will keep the problem for extenders, as parts of the classes they extend could change.
Yes, there is still a lot of point A (making internal API public) that the patch doesn't cover. (I didn't include the rest of the making-internal-API-public changes because they are trivial to make, and with the warnings in this patch it is now easy to see which of the remaining API is required - most of it at least) But answering your question, yes, I would definitely like that all necessary public API be exposed, not just in the GDB-DSF plugin, but the DSF plugin as well. Even org.eclipse.cdt.debug.ui has some API that I'd like to see public (although its for a different UI code component - it could probably be discussed separately). It's not just a matter of fixing warnings for aesthetic purposes, but to guarantee that IDEs that depend on CDT in this way will not break with service or minor version CDT upgrades.
(In reply to Bruno Medeiros from comment #7) > Yes, there is still a lot of point A (making internal API public) that the > patch doesn't cover. (I didn't include the rest of the > making-internal-API-public changes because they are trivial to make, and > with the warnings in this patch it is now easy to see which of the remaining > API is required - most of it at least) Ok, so I think it is safe to continue forward with this effort. Can you post a patch that addresses a complete part of the new API? It does not have to address all the changes we'd like in the end, but it would be nice to have the entire chain of classes affected by the patch included in it.
Sure, I can add that to the patch. But (for the patch purposes at least), what would be the preferred way to expose the new API classes? Just move them to a new public package? Or also create deprecated classes, in the internal packages, that extend the new public classes?
(In reply to Bruno Medeiros from comment #9) > Sure, I can add that to the patch. But (for the patch purposes at least), > what would be the preferred way to expose the new API classes? Just move > them to a new public package? First let's move them. That will make the review easier and will keep the git history across the renaming. > Or also create deprecated classes, in the > internal packages, that extend the new public classes? We'll do that as a second patch, once the first one is in, to avoid messing up the history in Git.
