Re: [cdt-dev] Customizing debug hierarchy and LaunchVMAdapter

[Bruno Medeiros <bruno.do.medeiros@xxxxxxxxx>]

These problems are similar to those I encountered in the course of addressing https://bugs.eclipse.org/bugs/show_bug.cgi?id=432323 .  The proposed changes (seen here https://git.eclipse.org/r/#/c/30274/ ) also try to remedy GdbAdapterFactory, so that we can extend GdbViewModelAdapter without duplicating the whole GdbAdapterFactory .

The solution I submitted is slightly different, but I think slightly better. With Vladimir's change, you have to extend GdbAdapterFactory to override createViewModelAdapter (that in itself is not a problem), but you still have to put code like this in the plugin.xml of the 3rd party CDT extension:

   <extension point="org.eclipse.core.runtime.adapters">
      <factory
            class="org.eclipse.cdt.examples.dsf.gdb.GdbExtendedAdapterFactory"
            adaptableType="org.eclipse.cdt.examples.dsf.gdb.launch.GdbExtendedLaunch">
         <adapter type="org.eclipse.debug.internal.ui.viewers.model.provisional.IElementContentProvider"/>
         <adapter type="org.eclipse.debug.internal.ui.viewers.model.provisional.IModelProxyFactory"/>
         <adapter type="org.eclipse.debug.ui.contexts.ISuspendTrigger"/>
         <adapter type="org.eclipse.debug.internal.ui.viewers.model.provisional.IColumnPresentationFactory"/>
      </factory>        
   </extension>

The problem here is the lines:
         <adapter type="org.eclipse.debug.internal.ui.viewers.model.provisional.IElementContentProvider"/>
         <adapter type="org.eclipse.debug.internal.ui.viewers.model.provisional.IModelProxyFactory"/>
         <adapter type="org.eclipse.debug.ui.contexts.ISuspendTrigger"/>
         <adapter type="org.eclipse.debug.internal.ui.viewers.model.provisional.IColumnPresentationFactory"/>

are internal *and* duplicated from CDT's plugin.xml extension. That's two problems. Being internal could be addressed easily if those classes are made public, but the duplication is a more serious problem. What if at some point some of these classes above are added or removed from the CDT's plugin.xml ? Then 3rd party CDT extensions will silently become out of date (or even worse, break out of nowhere)

The way that was done in https://git.eclipse.org/r/#/c/30274/ , a new adapter type is introduced specifically for the purpose of extending GdbViewModelAdapterFactory, so the 3rd party plugin.xml looks like this :

   <extension point="org.eclipse.core.runtime.adapters">
      <factory
            class="melnorme.lang.ide.debug.ui.GdbViewModelAdapterFactory"
            adaptableType="org.dsource.ddt.debug.core.DeeGdbLaunch">
         <adapter type="org.eclipse.cdt.dsf.gdb.ui.viewmodel.IGdbViewModelServicesFactory"/>
      </factory>
   </extension>

BTW, the full example of a CDT extension using the Gerrit change proposal above can be seen here:

https://github.com/bruno-medeiros/DDT/tree/new_CDT_API/plugin_ide.debug

On Fri, Sep 12, 2014 at 2:13 PM, Marc Khouzam <marc.khouzam@xxxxxxxxxxxx> wrote:

The patch looks interesting in its simplicity.
Because of the 8.5 release I won't have time to try it right away.
Could you modify the patch on the Gerrit review to use your suggestion
and see if it works?

Thanks!

> -----Original Message-----
> From: cdt-dev-bounces@xxxxxxxxxxx [mailto:cdt-dev-bounces@xxxxxxxxxxx]
> On Behalf Of Vladimir Prus

> Sent: Friday, September 12, 2014 4:47 AM
> To: cdt-dev@xxxxxxxxxxx
> Subject: Re: [cdt-dev] Customizing debug hierarchy and LaunchVMAdapter
>
>
> Yeah, GdbExtendedAdapterFactory is 400 lines of copy-paste ;-)
>
> I did succeed in extending it without repeating code, per patch below - is it
> about right?
>
> Note that so far I've tweaked label for threads - so I might run into more
> troubles further.
>
> - Volodya
>
> On 09/12/2014 12:24 AM, Marc Khouzam wrote:
> > This is exactly what I was hoping to figure out.  I've posted an example that
> extends DSF-GDB.
> > This is how I got things to work, but this is my first try at actually extending
> it.
> > http://eclip.se/441277
> >
> > I also had a lot of trouble extending GdbAdapterFactory.
> >
> > I was hoping others could provide feedback on what Vladimir is
> > describing below to see if there is a better way as of now.
> >
> >  From what I tried, we are good at extending the DSF services and
> > handling GDB versions, but we need to improve on allowing extension of
> the view model.
> >
> > Vladimir seems to have experienced the same results.
> >
> > Anyone else?
>
> Index:
> org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/GdbAdapt
> erFactory.java
> ==========================================================
> =========
> ---
> org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/GdbAdapt
> erFactory.java        (revision 436379)
> +++
> org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/GdbAdapt

> erFactory.java        (working copy)

> @@ -70,6 +70,7 @@
>   import org.eclipse.cdt.dsf.gdb.launching.GdbLaunch;
>   import org.eclipse.cdt.dsf.gdb.launching.GdbLaunchDelegate;
>   import org.eclipse.cdt.dsf.service.DsfSession;
> +import org.eclipse.cdt.dsf.ui.viewmodel.AbstractVMAdapter;
>   import org.eclipse.cdt.ui.text.c.hover.ICEditorTextHover;
>   import org.eclipse.core.runtime.IAdapterFactory;
>   import org.eclipse.debug.core.DebugPlugin;
> @@ -106,7 +107,7 @@
>       @Immutable
>       class SessionAdapterSet {
>           final GdbLaunch fLaunch;
> -        final GdbViewModelAdapter fViewModelAdapter;
> +        final AbstractVMAdapter fViewModelAdapter;
>           final DsfSourceDisplayAdapter fSourceDisplayAdapter;
>           final DsfStepIntoCommand fStepIntoCommand;
>           final DsfStepIntoSelectionCommand fStepIntoSelectionCommand; @@
> -147,7 +148,7 @@
>               fSteppingController = new SteppingController(session);
>               session.registerModelAdapter(SteppingController.class,
> fSteppingController);
>
> -            fViewModelAdapter = new GdbViewModelAdapter(session,
> fSteppingController);
> +            fViewModelAdapter = createViewModelAdapter(session,
> + fSteppingController);
>               session.registerModelAdapter(IViewerInputProvider.class,
> fViewModelAdapter);
>
>               if (launch.getSourceLocator() instanceof ISourceLookupDirector) {
> @@ -429,4 +430,9 @@
>       public void launchesChanged(ILaunch[] launches) {
>       }
>
> +    protected AbstractVMAdapter createViewModelAdapter(DsfSession
> session, SteppingController steppingController)
> +    {
> +     return new GdbViewModelAdapter(session, steppingController);
> +    }
> +
>   }
> Index:
> org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/viewmod
> el/launch/LaunchVMProvider.java
> ==========================================================
> =========
> ---
> org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/viewmod
> el/launch/LaunchVMProvider.java       (revision 436379)
> +++
> org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/viewmod

> el/launch/LaunchVMProvider.java       (working copy)

> @@ -57,7 +57,11 @@
>       {
>           super(adapter, presentationContext, session);
>
> -        IRootVMNode launchNode = new LaunchRootVMNode(this);
> +        createNodes();
> +    }
> +
> +     protected void createNodes() {
> +             IRootVMNode launchNode = new LaunchRootVMNode(this);
>           setRootNode(launchNode);
>
>           // Container node to contain all processes and threads @@ -70,7 +74,7
> @@
>
>           IVMNode stackFramesNode = new StackFramesVMNode(this,
> getSession());
>           addChildNodes(threadsNode, new IVMNode[] { stackFramesNode });
> -    }
> +     }
>
>       @Override
>       protected boolean canSkipHandlingEvent(Object newEvent, Object
> eventToSkip) {
>
>
> --
> Vladimir Prus
> CodeSourcery / Mentor Graphics
> http://www.mentor.com/embedded-software/
> _______________________________________________
> cdt-dev mailing list
> cdt-dev@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe
> from this list, visit https://dev.eclipse.org/mailman/listinfo/cdt-dev
_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/cdt-dev

--

Bruno Medeiros
https://twitter.com/brunodomedeiros