Bug 331670 - "Workspace Cannot Be Locked" dialog should have an edit button
Summary: "Workspace Cannot Be Locked" dialog should have an edit button
Status: VERIFIED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.7   Edit
Hardware: PC Windows 7
: P3 enhancement (vote)
Target Milestone: 3.7 M4   Edit
Assignee: Curtis Windatt CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2010-12-02 09:21 EST by Markus Keller CLA
Modified: 2010-12-07 11:40 EST (History)
2 users (show)

See Also:
curtis.windatt.public: review+


Attachments
Implementation (19.01 KB, patch)
2010-12-06 13:23 EST, Markus Keller CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2010-12-02 09:21:31 EST
HEAD

When you try to launch an Eclipse Application or JUnit Plug-in Test launch configuration and the workspace is already in use, you get a "Workspace Cannot Be Locked" dialog with just an OK button.

That dialog should contain a second button "Edit...", which opens the launch configuration dialog and selects the current workspace.


I quickly looked into the implementation:

LauncherUtils.WORKSPACE_LOCKED is used to trigger the dialog from LauncherUtils#clearWorkspace(ILaunchConfiguration, String, IProgressMonitor) and show it in LauncherUtilsStatusHandler#handleStatus(IStatus, Object). For the edit button action, we need the launch configuration and the launch mode, but the passed 'source' object is just the workspace name.

=> Do you expect that there are third-party clients that rely on getting just the workspace name in the LauncherUtils.WORKSPACE_LOCKED message?
If yes, then we need to hack something up to maintain backwards compatibility.
Comment 1 Curtis Windatt CLA 2010-12-02 12:14:55 EST
(In reply to comment #0)
> => Do you expect that there are third-party clients that rely on getting just
> the workspace name in the LauncherUtils.WORKSPACE_LOCKED message?
> If yes, then we need to hack something up to maintain backwards compatibility.

It is possible that a third party app uses our launcher with a different status handler.  However, the status and object sent to the handler is not API.  In fact the possible statuses that we might send to the handler is not defined anywhere, which would make it difficult for someone to implement their own.  The typical reason why status handlers are overridden is to avoid opening UI and I have never heard of anyone trying to self-host without a UI.

I don't think that modifying the object will be a problem.
Comment 2 Markus Keller CLA 2010-12-06 13:23:36 EST
Created attachment 184645 [details]
Implementation

Here an implementation. Would be nice if it could make it into M4, since it would complete the workflow with the new options for the default workspaces.

The patch has 2 little problems (but IMO, they shouldn't prevent you from releasing it):

- The Edit... button just opens the launch config dialog and selects the launch. The perfect solution would also make sure it opens on the Main tab and has the workspace data field selected. I didn't find an easy way to achieve that, and hacking something with the status in DebugUITools#openLaunchConfigurationDialogOnGroup(..., IStatus) seemed too advanced.

- The static LauncherUtils#fLastLaunchMode is not a beauty, but that was the easiest way I found to pass on the information. Unfortunately, I couldn't add the launch mode or the ILaunch to AbstractPDELaunchConfiguration#clear(ILaunchConfiguration, IProgressMonitor), since that method is API that can be called and overridden by clients.
Comment 3 Curtis Windatt CLA 2010-12-06 15:30:22 EST
Fixed in HEAD.  Only change I made was to add some javadocs to LauncherUtils to explain the option.
Comment 4 Dani Megert CLA 2010-12-07 04:45:44 EST
This is not consistently working, see bug 331989 for details.
Comment 5 Dani Megert CLA 2010-12-07 11:40:34 EST
> - The Edit... button just opens the launch config dialog and selects the
> launch. The perfect solution would also make sure it opens on the Main tab and
> has the workspace data field selected. I didn't find an easy way to achieve
> that, and hacking something with the status in
> DebugUITools#openLaunchConfigurationDialogOnGroup(..., IStatus) seemed too
> advanced.
I must say that I missed that, especialyl because all other places in the SDK where we click a link or go to some other dialog, we do select the right field. I've filed bug 332047 to track this.