Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 339550

Summary:

[launch] Allow to override GdbLaunch

Product:

[Tools] CDT

Reporter:

Nobody - feel free to take it <nobody>

Component:

cdt-debug-dsf-gdb

Assignee:

Marc Khouzam <marc.khouzam>

Status:

RESOLVED FIXED

QA Contact:

Marc Khouzam <marc.khouzam>

Severity:

enhancement

Priority:

CC:

abeerbagul, cdtdoug, pawel.1.piech, zulliger

Version:

8.0

Flags:

nobody: review+

Target Milestone:

8.1.0

Hardware:

OS:

All

Whiteboard:

Attachments:

Description	Flags
Abeer bagul's patch from 326951	none
Slightly updated fix to add parameters to new API	marc.khouzam: iplog-
Cleanup to remove global variables	marc.khouzam: iplog-
non functional patch for iplog purposes.	marc.khouzam: iplog+

Description Nobody - feel free to take it

2011-03-10 11:57:57 EST

Created attachment 190883 [details]
Abeer bagul's patch from 326951

Submission from Abeer Bagul, copied from https://bugs.eclipse.org/bugs/show_bug.cgi?id=326951.
When we (in the Xtensa Xplorer IDE) create a subclass of GDBLaunchDelegate and
a subclass of GDBLaunch, my intention is just to return the object of the
GDBLaunch subclass from the getLaunch() method. 
I would not like to override the entire getLaunch() method, as this forces me
to override the launch object initialization sequence also.
If the getLaunch() method can call a createLaunch() method to get the launch
object, and the createLaunch() method is protected, then we can just override
createLaunch(), return an object of GDBLaunch subclass, and have the
getLaunch() method do the initialization of the launch object.

If I want to override the initialization sequence of the launch object,
override getLaunch() as well as createLaunch().

The same applies to GDBLaunchDelegate.getSourceLocator(). This should delegate
object creation to a protected createSourceLocator(), which just returns a new
DSFSourceLocator. Subclasses of GDBLaunchDelegate can just override
createSourceLocator, or override getSourceLocator() too if they want to change
the initialization sequence of the sourcelocator object.

Am attaching the modified patch with addition of two methods:
createLaunch()
createSourceLocator() 
and calling these from getLaunch and getSourceLocator respectively.

Comment 1 Marc Khouzam

2011-08-05 13:44:11 EDT

(In reply to comment #0)
> Created attachment 190883 [details]
> Abeer bagul's patch from 326951

Abeer, can you re-attach the patch yourself?  I need that to set the iplog flag on your name.

Thanks.

Comment 2 Marc Khouzam

2011-08-05 13:52:37 EDT

Created attachment 201000 [details]
Slightly updated fix to add parameters to new API

This is Abeer's patch except that I have
1- renamed createLaunch() to createGdbLaunch() since it must return a GdbLaunch
2- renamed createSourceLocator() to createDsfSourceLocator() since it must return a DsfSourceLookupDirector
3- added an ISourceLocator parameter to createGdbLaunch().  GdbLaunch requires a ISourceLocator as a parameter and although we currently pass null, we may need to pass a real one in the future.
4- added an ILaunchConfiguration parameter to createDsfSourceLocator(). It seemed more future-proof.

I'll commit to master.

Comment 3 Marc Khouzam

2011-08-05 14:09:41 EDT

Created attachment 201002 [details]
Cleanup to remove global variables

Here is a patch that cleans up GdbLaunchDelegate a little to make it even easier to override.  The patch removes the need for the annoying global variables and relies on the ILaunchConfiguration instead.

For backwards-compatibility, I had to keep newServiceFactory(String) but I deprecated it.  There is a new newServiceFactory(ILaunchConfiguration,String) to use instead.  To keep the deprecated one working as expected though, I had to keep the global fIsNonStopSession, but new users won't need it anymore.

I'll commit this to master as well.

Comment 4 Marc Khouzam

2011-08-05 14:17:43 EDT

Mikhail, what do you think of this change?

Abeer, can you confirm this achieves what you wanted? (and attach the patch yourself so I can add the iplog flag)

Comment 5 CDT Genie

2011-08-05 14:23:02 EDT

*** cdt git genie on behalf of Marc Khouzam ***

    Bug 339550: Allow to override GdbLaunch

[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=0d2872027c4075eb4e2c583ebabce6b983a72567

Comment 6 CDT Genie

2011-08-05 14:23:04 EDT

*** cdt git genie on behalf of Marc Khouzam ***

    Bug 339550: Cleanup GdbLaunchDelegate for easier overriding

[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=0f94c41a730ebaebc2f05fed27e509af796b6007

Comment 7 Abeer Bagul

2011-08-09 09:11:03 EDT

Created attachment 201139 [details]
non functional patch for iplog purposes.

I have reattached the patch from attachment 201000 [details], since it is required for iplog purposes. 
Please do not use this patch for patching purposes, use attachment 201000 [details] instead.

Comment 8 Nobody - feel free to take it

2011-08-10 10:39:19 EDT

Looks good.