Bug 572756

Summary: MultiPageEditorPart is hard to close programmatically
Product: [Eclipse Project] Platform Reporter: Ed Willink <ed>
Component: UIAssignee: Platform-UI-Inbox <Platform-UI-Inbox>
Status: NEW --- QA Contact:
Severity: normal    
Priority: P3 CC: rolf.theunissen
Version: 4.19   
Target Milestone: ---   
Hardware: PC   
OS: Windows 10   
Whiteboard:

Description Ed Willink CLA 2021-04-11 04:05:20 EDT
A comment within the package private MultiPageEditorPart.close() is

    // 3.x implementation closes the editor when the ISL is disposed

which is hardly helpful, and actually wrong. Debugging the tool bar close of the UML Editor shows that close() is not called and little use of a Service Locator.

Certainly dispose()ing just the editor does nothing till another view changes and causes the UI to catch up. dispose()ing the site seems better but can get NPE from actions.

The correct approach would appear to be:
				multiPartEditor.getSite().getPage().closeEditor(multiPartEditor, false);

Surely this should at least appear in some Javadoc and probably be the publicly visible implementation of close()?
Comment 1 Rolf Theunissen CLA 2021-04-11 09:56:57 EDT
The close method is an internal method that is used to close the MultiPageEditorPart in case the ServiceLocator (ISL) of the multipage editor is disposed. This to be compatible with 3.x behavior.
The comment is there for platform developers to understand why this code is there.

The close method is by no means public API, it should never be called by client code (package private should be a clear hint). We could argue that the method name is not the clearest, but it isn't API anyhow.

The API offered to close any editor has always been IWorkbenchPage#closeEditor, see
https://help.eclipse.org/2021-03/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Freference%2Fapi%2Forg%2Feclipse%2Fui%2FIWorkbenchPage.html&anchor=closeEditor(org.eclipse.ui.IEditorPart,boolean)

I am not sure what problem you have with the current implementation?
Comment 2 Ed Willink CLA 2021-04-11 11:33:07 EDT
(In reply to Rolf Theunissen from comment #1)
> The API offered to close any editor has always been
> IWorkbenchPage#closeEditor

Except that it is also ITextEditor.close()

If you have an editor and want to close it, you expect to invoke a method on it, not go round the houses to find suitable conspirators. If the framework needs to go round the houses, then a close() implementation can do that for the user.

so as per the report a close() implementation should do the obvious job.

else as per the report the close() method should be helpfully documented.