Bug 318642

Comment 1 Anna Dushistova 2010-07-01 15:12:30 EDT

Created attachment 173255 [details]
patch that fixes the problem

Comment 2 Nobody - feel free to take it 2010-07-02 12:14:05 EDT

I would recommend using accessor instead of making an internal field API.

Comment 3 Anna Dushistova 2010-07-03 02:21:13 EDT

Created attachment 173343 [details]
another variant

There is already a method isNonStopSession(), it has nothing to do with the field though, but for my purposes that will do too. Just seems weird to me to have both a getter and isNonStopSession()...

Comment 4 Marc Khouzam 2010-07-06 08:39:13 EDT

The code in GdbLaunchDelegate to handle NonStop is not very nice and does not allow for proper overriding.  There may be a better way to do this than to simply make those method visible.

What exactly do you need to do?  With an example of overriding needs, we may come up with a cleaner solution.

Comment 5 Anna Dushistova 2010-07-07 07:46:58 EDT

I want to use my own service factory, which is extended GdbDebugServiceFactory class. I also want my launch to support non stop mode if user wishes so.
Here is a code snippet of what I would like to be able to do:
	@Override
	protected IDsfDebugServicesFactory newServiceFactory(String version) {
		if (isNonStopSession && isNonStopSupported(version)) {
			return new MyGdbDebugServicesFactoryNS(version);
		}
		return new MyGdbDebugServicesFactory(version);
	}

Comment 6 Marc Khouzam 2010-07-08 16:04:13 EDT

Created attachment 173812 [details]
Adapted fix

I cannot find a nice way to solve this without chaning newServiceFactory(version) to also take a launch configuration as a parameter.

So, this patch suggests some renaming and moving some methods to LaunchUtils.

Like your patch, I didn't make fIsNonStopSession (was isNonStopSession) visible, or gave it a getter.  I think this is too specific.  What you will need to do is create your own field and override getLaunch() to set that field and then call super.getLaunch()

I haven't committed it yet.  Is this good enough?

Mikhail, if you have a more elegant solution, that would be good :-)

Comment 7 Nobody - feel free to take it 2010-07-08 16:33:56 EDT

(In reply to comment #6)
> Mikhail, if you have a more elegant solution, that would be good :-)

Marc, I have already had a big task backlog. If I get fired, I promise to focus entirely on CDT :)

Comment 8 Anna Dushistova 2010-07-08 16:55:43 EDT

(In reply to comment #6)
> Created an attachment (id=173812) [details]
> Adapted fix
> 
> I cannot find a nice way to solve this without chaning
> newServiceFactory(version) to also take a launch configuration as a parameter.
> 
> So, this patch suggests some renaming and moving some methods to LaunchUtils.
> 
> Like your patch, I didn't make fIsNonStopSession (was isNonStopSession)
> visible, or gave it a getter.  I think this is too specific.  What you will
> need to do is create your own field and override getLaunch() to set that field
> and then call super.getLaunch()
> 
> I haven't committed it yet.  Is this good enough?

Not what I dreamed of, but can work.

Comment 9 Marc Khouzam 2010-07-09 08:05:28 EDT

(In reply to comment #7)
> (In reply to comment #6)
> > Mikhail, if you have a more elegant solution, that would be good :-)
> 
> Marc, I have already had a big task backlog. If I get fired, I promise to focus
> entirely on CDT :)

:-)
I just wanted you to feel free to overrule my suggestion.

Comment 10 CDT Genie 2010-07-09 08:23:03 EDT

*** cdt cvs genie on behalf of mkhouzam ***
Bug 318642: Make GDBLaunchDelegate easier to override with respect to non-stop

[*] LaunchUtils.java 1.13 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/LaunchUtils.java?root=Tools_Project&r1=1.12&r2=1.13
[*] GdbLaunchDelegate.java 1.17 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/GdbLaunchDelegate.java?root=Tools_Project&r1=1.16&r2=1.17

Comment 11 Marc Khouzam 2010-07-09 08:30:06 EDT

(In reply to comment #8)
> > I haven't committed it yet.  Is this good enough?
> 
> Not what I dreamed of, but can work.

Committed to HEAD