Bug 572756 - MultiPageEditorPart is hard to close programmatically
Summary: MultiPageEditorPart is hard to close programmatically
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.19   Edit
Hardware: PC Windows 10
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-04-11 04:05 EDT by Ed Willink CLA
Modified: 2021-04-11 11:33 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.