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

Bug 214696

Summary: refactor WorkingDirectoryBlock
Product: [Eclipse Project] JDT Reporter: Remy Suen <remy.suen>
Component: DebugAssignee: JDT-Debug-Inbox <jdt-debug-inbox>
Status: VERIFIED FIXED QA Contact:
Severity: enhancement    
Priority: P3 CC: darin.eclipse, d_a_carver
Version: 3.4   
Target Milestone: 3.4 M5   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
Patch to remove "most" of the JDT Debug dependencies on the WorkingDirectoryBlock class.
none
Updated patch to also patch o.e.ant.ui.
none
Updated patch to prevent an NPE from being thrown. none

Description Remy Suen CLA 2008-01-08 18:18:09 EST
It would be really nice if WorkingDirectoryBlock was exposed as an API class so that plug-in developers using the debug APIs would not have to copy/paste this code around by hand.
Comment 1 Darin Wright CLA 2008-01-16 09:11:49 EST
Not planned for 3.4
Comment 2 Remy Suen CLA 2008-01-20 17:50:12 EST
Created attachment 87364 [details]
Patch to remove "most" of the JDT Debug dependencies on the WorkingDirectoryBlock class.

This doesn't make the class API but it removes all of the dependencies on the WorkingDirectoryBlock class except for SWTFactory and also altered it into an abstract class.

Could the debug team comment on this? Thanks.
Comment 3 Darin Wright CLA 2008-01-21 12:34:35 EST
Looks like a reasonable start. Note that this breaks org.eclipse.ant.ui, that also extends the working directory block. Perhaps you want to update it to work within the new framework?
Comment 4 Remy Suen CLA 2008-01-22 15:25:42 EST
Created attachment 87556 [details]
Updated patch to also patch o.e.ant.ui.

(In reply to comment #3)
> Looks like a reasonable start. Note that this breaks org.eclipse.ant.ui, that
> also extends the working directory block. Perhaps you want to update it to work
> within the new framework?

Your wish is my command.

In this patch, besides the update to o.e.ant.ui, I also made the setEnabled(boolean) method public and added an assertion check in the constructor to ensure that the attribute name isn't null.
Comment 5 Darin Wright CLA 2008-01-22 16:51:58 EST
Thanks, will consider for 3.4
Comment 6 Darin Wright CLA 2008-01-23 15:57:53 EST
With the patch I get the following NPE in this case:

* Create a project
* Open its "Properties..."
* Select the "Builder" page
* Press "New..."
* Select "Ant" in the list and press OK

java.lang.NullPointerException
    at org.eclipse.jdt.internal.debug.ui.launcher.JavaWorkingDirectoryBlock.getProject(JavaWorkingDirectoryBlock.java:31)
    at org.eclipse.jdt.internal.debug.ui.launcher.WorkingDirectoryBlock.setDefaultWorkingDir(WorkingDirectoryBlock.java:288)
    at org.eclipse.ant.internal.ui.launchConfigurations.AntWorkingDirectoryBlock.setDefaultWorkingDir(AntWorkingDirectoryBlock.java:41)
    at org.eclipse.ant.internal.ui.launchConfigurations.AntWorkingDirectoryBlock.initializeFrom(AntWorkingDirectoryBlock.java:58)
    at org.eclipse.ant.internal.ui.launchConfigurations.AntJRETab.initializeFrom(AntJRETab.java:202)
    at org.eclipse.debug.ui.AbstractLaunchConfigurationTabGroup.initializeFrom(AbstractLaunchConfigurationTabGroup.java:86)
    at org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationTabGroupWrapper.initializeFrom(LaunchConfigurationTabGroupWrapper.java:143)
    at org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationTabGroupViewer.displayInstanceTabs(LaunchConfigurationTabGroupViewer.java:786)
    at org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationTabGroupViewer$8.run(LaunchConfigurationTabGroupViewer.java:662)
    at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:67)
    at org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationTabGroupViewer.inputChanged(LaunchConfigurationTabGroupViewer.java:679)
    at org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationTabGroupViewer.setInput0(LaunchConfigurationTabGroupViewer.java:641)
    at org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationTabGroupViewer.setInput(LaunchConfigurationTabGroupViewer.java:617)
    at org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationPropertiesDialog.initializeContent(LaunchConfigurationPropertiesDialog.java:128)
    at org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationsDialog.createContents(LaunchConfigurationsDialog.java:442)
    at org.eclipse.jface.window.Window.create(Window.java:431)
    at org.eclipse.jface.dialogs.Dialog.create(Dialog.java:1088)
    at org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationsDialog.create(LaunchConfigurationsDialog.java:371)
    at org.eclipse.jface.window.Window.open(Window.java:790)
    at org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationsDialog.open(LaunchConfigurationsDialog.java:1113)
    at org.eclipse.debug.internal.ui.launchConfigurations.LaunchConfigurationPropertiesDialog.open(LaunchConfigurationPropertiesDialog.java:229)
    at org.eclipse.debug.ui.DebugUITools.openLaunchConfigurationPropertiesDialog(DebugUITools.java:435)
    at org.eclipse.debug.ui.DebugUITools.openLaunchConfigurationPropertiesDialog(DebugUITools.java:414)
    at org.eclipse.ui.externaltools.internal.ui.BuilderPropertyPage.editConfiguration(BuilderPropertyPage.java:619)
    at org.eclipse.ui.externaltools.internal.ui.BuilderPropertyPage.handleNewButtonPressed(BuilderPropertyPage.java:584)
    at org.eclipse.ui.externaltools.internal.ui.BuilderPropertyPage.handleButtonPressed(BuilderPropertyPage.java:392)
    at org.eclipse.ui.externaltools.internal.ui.BuilderPropertyPage.access$0(BuilderPropertyPage.java:390)
    at org.eclipse.ui.externaltools.internal.ui.BuilderPropertyPage$1.widgetSelected(BuilderPropertyPage.java:129)
    at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:227)
    at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:66)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:952)
    at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3758)
    at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3369)
    at org.eclipse.jface.window.Window.runEventLoop(Window.java:825)
    at org.eclipse.jface.window.Window.open(Window.java:801)
    at org.eclipse.ui.dialogs.PropertyDialogAction.run(PropertyDialogAction.java:156)
    at org.eclipse.jface.action.Action.runWithEvent(Action.java:498)
    at org.eclipse.jface.action.ActionContributionItem.handleWidgetSelection(ActionContributionItem.java:582)
    at org.eclipse.jface.action.ActionContributionItem.access$2(ActionContributionItem.java:499)
    at org.eclipse.jface.action.ActionContributionItem$5.handleEvent(ActionContributionItem.java:410)
    at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:66)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:952)
    at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3758)
    at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3369)
    at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2392)
    at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2356)
    at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2222)
    at org.eclipse.ui.internal.Workbench$4.run(Workbench.java:474)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:288)
    at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:469)
    at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
    at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:106)
    at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:193)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:106)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:76)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:362)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:175)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
    at java.lang.reflect.Method.invoke(Method.java:597)
    at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:564)
    at org.eclipse.equinox.launcher.Main.basicRun(Main.java:504)
    at org.eclipse.equinox.launcher.Main.run(Main.java:1251)
    at org.eclipse.equinox.launcher.Main.main(Main.java:1227)
Comment 7 Remy Suen CLA 2008-01-23 18:51:28 EST
Created attachment 87717 [details]
Updated patch to prevent an NPE from being thrown.

How embarassing.
Comment 8 Darin Wright CLA 2008-01-28 10:53:36 EST
Applied patch with some javadoc updates.
Comment 9 Darin Wright CLA 2008-01-28 10:54:15 EST
Verified.
Comment 10 Remy Suen CLA 2008-01-28 13:07:17 EST
Hi Darin, thanks for committing the patch. Should I now open an alternate bug for making the WorkingDirectoryBlock public API or does the Platform/Debug team have no plans of supporting it?
Comment 11 Remy Suen CLA 2008-02-06 09:07:55 EST
Hi, I was wondering if someone from the debug team could reply to comment 10?
Comment 12 Darin Wright CLA 2008-02-06 10:41:07 EST
So why do you want to use the block? We provide the tabs as API to be re-used, but we're less excited about exposing their individual parts as API. The class itself looks reasonable but then there is a waterfall of other blocks to expose...
Comment 13 Remy Suen CLA 2008-02-06 12:41:21 EST
(In reply to comment #12)
> So why do you want to use the block?

Well, as you would imagine, I need the block exposed so that I can let my users set a working directory for my launch configurations (I am launching .NET executables and not Java applications). I'd imagine that anyone using java.lang.Runtime's exec method for launching would want to let their users set a 'working directory'.
Comment 14 Remy Suen CLA 2008-02-27 09:54:23 EST
Hi Darin, do you understand my use case per comment 13?
Comment 15 David Carver CLA 2008-03-01 18:24:11 EST
(In reply to comment #12)
> So why do you want to use the block? We provide the tabs as API to be re-used,
> but we're less excited about exposing their individual parts as API. The class
> itself looks reasonable but then there is a waterfall of other blocks to
> expose...
> 

See bug 221042 for another use case to have it as API as well.    In our case, there are times when having the ability to specify the working directory that default files are outputed to is advantageous.  It doesn't force the user developing some xslt 1.0 files that use the exslt:document function, to have to basically create a variable to be set to hold the working directory.
Comment 16 Darin Wright CLA 2008-03-04 11:36:10 EST
> Well, as you would imagine, I need the block exposed so that I can let my users
> set a working directory for my launch configurations (I am launching .NET
> executables and not Java applications). I'd imagine that anyone using
> java.lang.Runtime's exec method for launching would want to let their users set
> a 'working directory'.

So in this case, the API should not exist in JDT Debug, but in the Debug platform. I.e. JDT is not a prereq for setting a working directory, it's just launching in general.
Comment 17 Remy Suen CLA 2008-03-04 14:38:41 EST
(In reply to comment #16)
> So in this case, the API should not exist in JDT Debug, but in the Debug
> platform. I.e. JDT is not a prereq for setting a working directory, it's just
> launching in general.

That's correct. I filed this under JDT/Debug initially to ensure that the JDT team was willing to at least refactor things since if they weren't interested in doing it, this block is never going to become API. ;)
Comment 18 Remy Suen CLA 2008-03-04 14:39:26 EST
Reiterating comment 10, should I open a new bug against Platform/Debug?
Comment 19 Darin Wright CLA 2008-03-04 14:52:07 EST
(In reply to comment #18)
> Reiterating comment 10, should I open a new bug against Platform/Debug?

Sure. Don't expect anything for 3.4 though (API freeze is in a couple weeks).

Comment 20 Remy Suen CLA 2008-03-08 22:29:04 EST
(In reply to comment #19)
> Sure. Don't expect anything for 3.4 though (API freeze is in a couple weeks).

Done, I have created bug 221973 for this. Thanks Darin.