Community
Participate
Working Groups
Created attachment 224597 [details] The "Save Resources" dialog box is missing from Eclipse 4.2.0 When you have multiple files open that have unsaved changes, right-clicking on the tabs area and selecting "Close All" used to bring up a "Save Resources" dialog that allows you to choose which files to save and/or save all unsaved files in one click (see attachment). With Juno (4.2.0), that dialog box is missing - instead you're given one dialog per unsaved file asking you if you want to save that file. With many unsaved files open, that's a lot of clicking and could result in saving the wrong file.
NOTE 1: one gets a dialog for each dirty editor. NOTE 2: it works when using File > Close All or Ctrl+Shift+W
I recommend moving this to 4.4 (Luna) - the work involved is too extensive for this late in 4.3 (Kepler). The details: The 'Save Resources' dialog is deep inside the legacy workbench. The editor tab menu is created/handled by the e4 StackRenderer. The e4 workbench cannot depend on the legacy workbench. This appears to be a good candidate (in 4.4) for refactoring so that an e4 application can use this functionality. Some notes (for next time): * The editor tab menu items (in 4.x) are created/handled by org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer * The 'Save Resources' dialog is a private, nested class org.eclipse.ui.internal.SaveablesList.MyListSelectionDialog in the org.eclipse.ui.workbench plugin. * Close All and Close Others are closely related - in 4.x, they are implemented through essentially the same code path. In 3.x, they were handled by 'actions' that called into a code path that ultimately resulted in the 'Save Resources' dialog. * The Close All command (org.eclipse.ui.file.closeAll) is not invoked by the 4.x editor tab menus. Nor was it invoked by the 3.x editor tab menus. It lacks any parametrization to handle Close Others. While the command is declared in org.eclipse.ui, then handler lives in org.eclipse.ui.workbench.
Paul, perhaps we could take a slightly different tack; can we change the code so that the e4 code does *not* close 'dirty' parts ?
(In reply to comment #3) > Paul, perhaps we could take a slightly different tack; can we change the > code so that the e4 code does *not* close 'dirty' parts ? -1 for that. Then we'd better just remove it from the tab. Can't we just trigger the closeAll command? That should trigger the same code as File > Close All (Ctrl+Shift+W).
I'd like to see the commands close/closeAll/closeOthers called in the stack renderer menu items. We might need to provide useful Eclipse4 handlers that work currently in such a way that the 4.x Workbench handlers will override them correctly. PW
OK by me, but it looks like there are too many potential issues to consider this for Kepler. It's looking to me like Luna's focus should be all about breaking down the e4 / IDE boundaries, perhaps it'd be worth opening a 'root' defect to capture 'features' like this one, allowing direct definition of e4 components within the workbench...sound like a good idea ??
(In reply to comment #6) > OK by me, but it looks like there are too many potential issues to consider > this for Kepler. > > It's looking to me like Luna's focus should be all about breaking down the > e4 / IDE boundaries, perhaps it'd be worth opening a 'root' defect to > capture 'features' like this one, allowing direct definition of e4 > components within the workbench...sound like a good idea ?? Sorry to sound naive, but what's the problem of just invoking the closeAll command? Shouldn't this be a no-brainer?
Dani, those menu items appear and can be used from e4 applications that don't have the compatibility layer. The various handler implementations and the corresponding dialogs etc are currently tightly bound to the IDE. Paul's comment #2 gives a small summary of the issues.
(In reply to comment #8) > Dani, those menu items appear and can be used from e4 applications that > don't have the compatibility layer. The various handler implementations and > the corresponding dialogs etc are currently tightly bound to the IDE. Paul's > comment #2 gives a small summary of the issues I still don't get, why that menu item can't simply invoke the closeAll command. What makes it so hard?
The tab menus, created in org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.createTabMenu(CTabFolder, MPart), should be able to be broken up into the command calls through the EHandlerService/ECommandService and 2 handlers that should contain the implementations (slightly adapted) for what's currently the SWT.Selection events. Then the MCommands and probably MHandlers should be defined in our LegacyIDE.e4.xmi file. It should just be a slight refactoring exercise. PW
(In reply to comment #10) > The tab menus, created in > org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer. > createTabMenu(CTabFolder, MPart), should be able to be broken up into the > command calls through the EHandlerService/ECommandService and 2 handlers > that should contain the implementations (slightly adapted) for what's > currently the SWT.Selection events. Then the MCommands and probably > MHandlers should be defined in our LegacyIDE.e4.xmi file. > > It should just be a slight refactoring exercise. > > PW What's wrong using the following code: CommandManager cm = part.getContext().get(CommandManager.class); Command c = cm.getCommand("org.eclipse.ui.file.closeAll"); //$NON-NLS-1$ c.executeWithChecks(new ExecutionEvent()); ? At least for me this closes all editors using the expected save dialog. Of course, this does not help us with closeOthers because said command is not enabled at that point.
(In reply to comment #11) > (In reply to comment #10) > What's wrong using the following code: > > CommandManager cm = part.getContext().get(CommandManager.class); You'd do the same thing with EHandlerService.executeHandler(*). And it would work in the Eclipse SDK. But it would break Close and Close All for Eclipse4 RCP applications, unless we also refactor the current SWT.Selection calls into Eclipse4 default handlers. PW
(In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #10) > > What's wrong using the following code: > > > > CommandManager cm = part.getContext().get(CommandManager.class); > > You'd do the same thing with EHandlerService.executeHandler(*). And it > would work in the Eclipse SDK. But it would break Close and Close All for > Eclipse4 RCP applications, unless we also refactor the current SWT.Selection > calls into Eclipse4 default handlers. > > PW Thanks for the lesson :-).
Having a go at this for RC1 using Paul & Dani's advice comments 10 through 12.
Pushed patch to Gerrit: https://git.eclipse.org/r/12840 Changed StackRenderer to use the following commands in its context menu: * org.eclipse.ui.file.close * org.eclipse.ui.file.closeAll * org.eclipse.ui.file.closeOthers If the command service is unavailable, the menu is not created. If individual commands are unavailable that menu item is not created. Added and tested default pure-E4 implementations of the commands.
(In reply to comment #15) > Pushed patch to Gerrit: https://git.eclipse.org/r/12840 Thanks for this. It looked like Close,Close All, and Close Others go through the Workbench code when running the SDK, and it works as expected. Some comments: 1) I was hoping that we could just make the execute call like you have, but if those 3 commands aren't defined in the application model then the menu items wouldn't show up in any Eclipse4 application's stack. Maybe we can add them to the model using a fragment (add the commands and the handlers at the MApplication level), although I hope that doesn't interfere with the Workbench creating the commands. The alternative is to directly call the handlers if the commands don't exist (more code): One possibility for the StackRenderer: if (closeableElements >= 2) { MenuItem menuItemOthers = new MenuItem(menu, SWT.NONE); menuItemOthers.setText(SWTRenderersMessages.menuCloseOthers); if (isCommandAvailable(COMMAND_FILE_CLOSE_OTHERS)) { menuItemOthers.addSelectionListener(new SelectionAdapter() { public void widgetSelected(SelectionEvent e) { invokeCloseCommand(COMMAND_FILE_CLOSE_OTHERS, parent, menu); } }); } else { menuItemOthers.addSelectionListener(new SelectionAdapter() { public void widgetSelected(SelectionEvent e) { invokeCloseHandler(HANDLER_FILE_CLOSE_OTHERS, parent, menu); } }); } where: private static final String HANDLER_FILE_CLOSE_OTHERS = "bundleclass://org.eclipse.e4.ui.workbench/org.eclipse.e4.ui.internal.workbench.handlers.CloseOtherPartssHandler"; //$NON-NLS-1$ and: protected void invokeCloseHandler(final String uri, final MElementContainer<MUIElement> parent, final Menu menu) { MPart part = (MPart) menu.getData(STACK_SELECTED_PART); if (parent.getSelectedElement() != part) { parent.setSelectedElement(part); } final IEclipseContext eContext = modelService .getContainingContext(part); Object obj = contribFactory.create(uri, eContext); ContextInjectionFactory.invoke(obj, Execute.class, eContext, null); } 2) CloseOtherPartssHandler has an extra 's' 3) when I tried Close on the javadoc view in the bottom stack in the SDK I got the following exception. Is the parent finding code not reliable for shared parts? !ENTRY org.eclipse.ui 4 0 2013-05-15 19:31:19.137 !MESSAGE Unhandled event loop exception !STACK 0 java.lang.IllegalArgumentException: The selected element org.eclipse.e4.ui.model.application.ui.basic.impl.PartImpl@5c2a915e (elementId: org.eclipse.jdt.ui.JavadocView, tags: [View, categoryTag:Java, active], contributorURI: null) (widget: ContributedPartRenderer$2 {}, renderer: org.eclipse.e4.ui.workbench.renderers.swt.ContributedPartRenderer@447536f2, toBeRendered: true, onTop: false, visible: true, containerData: null, accessibilityPhrase: null) (contributionURI: bundleclass://org.eclipse.ui.workbench/org.eclipse.ui.internal.e4.compatibility.CompatibilityView, object: org.eclipse.ui.internal.e4.compatibility.CompatibilityView@352dac72, context: PartImpl (org.eclipse.jdt.ui.JavadocView) Context, variables: [], label: Javadoc, iconURI: platform:/plugin/org.eclipse.jdt.ui/icons/full/eview16/javadoc.gif, tooltip: , dirty: false, closeable: true, description: null) is not a child of this container at org.eclipse.e4.ui.model.application.ui.impl.ElementContainerImpl.setSelectedElement(ElementContainerImpl.java:158) at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.invokeCloseCommand(StackRenderer.java:1501) at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer$17.widgetSelected(StackRenderer.java:1332) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:248) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1392) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3742) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3363) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1113) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:997) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:138) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:610) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:567) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:150) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:124) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:354) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:181) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:601) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:636) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:591) at org.eclipse.equinox.launcher.Main.run(Main.java:1450) at org.eclipse.equinox.launcher.Main.main(Main.java:1426)
The change is quite big for RC1. Given there is a workaround (==> readme), I suggest to defer to 4.4, and if the fix works well in the field, we can backport it.
Deferring to Luna seems like a good idea. The failure seen with closing views is likely because the workbench's org.eclipse.ui.file.close handler was not expecting view parts. I need to investigate more.
(In reply to comment #15) > Pushed patch to Gerrit: https://git.eclipse.org/r/12840 > > Changed StackRenderer to use the following commands in its context menu: > * org.eclipse.ui.file.close Maybe we need to use org.eclipse.ui.file.closePart instead of o.e.ui.file.close for the single tab case. PW
There are a number of problems with the 're-use existing commands' approach: * The Close All and Close Others handlers work only on editors. * Furthermore, they consider on all open editors - they do not limit their scope to a particular part stack. It seems to me that we need a new 'close parts in stack' command that is view/editor agnostic, and that limits itself to the active part stack. Until the IDE's file save handling code (including the Save Resources dialog) is made pure e4, we'd still need an IDE-specific handler, and a generic (and less capable) pure-e4 handler. OR... A completely different approach: have an IDE-specific version of StackRenderer that can safely call legacy IDE code. Moving this to 4.4 - too much work to consider as save for 4.3.1
Pushed a proposed fix to Gerrit. It takes advantage of an ISaveHandler, if found in the context, to present the correct save dialog. Question: Should this functionality be pushed to a more public API, such as EPartService? https://git.eclipse.org/r/21774
Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=14734da604e6240048c4d62ef1279c9c7787d6a2 PW
Now it shows the list for 'Close Others' even there's only one file.
(In reply to comment #23) > Now it shows the list for 'Close Others' even there's only one file. Amended fix pushed to Gerrit for review: https://git.eclipse.org/r/21943
(In reply to Paul Elder from comment #24) > (In reply to comment #23) > > Now it shows the list for 'Close Others' even there's only one file. > > Amended fix pushed to Gerrit for review: > > https://git.eclipse.org/r/21943 Submitted with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=c42d61dbe5ead0fa4bb0f1fc60267d9003ff00d1
In 4.4.0.I20140303-2000 PW
*** Bug 440747 has been marked as a duplicate of this bug. ***
