| Summary: | refactor WorkingDirectoryBlock | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Remy Suen <remy.suen> |
| Component: | Debug | Assignee: | 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
Remy Suen
Not planned for 3.4 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.
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? 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. Thanks, will consider for 3.4 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)
Created attachment 87717 [details]
Updated patch to prevent an NPE from being thrown.
How embarassing.
Applied patch with some javadoc updates. Verified. 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? Hi, I was wondering if someone from the debug team could reply to comment 10? 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... (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'. Hi Darin, do you understand my use case per comment 13? (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. > 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.
(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. ;) Reiterating comment 10, should I open a new bug against Platform/Debug? (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). (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. |