Skip to content

Make CodeEditorWidget predicate injectable#8672

Merged
amiramw merged 1 commit intomasterfrom
CodeEditorWidgetUtil
Oct 28, 2020
Merged

Make CodeEditorWidget predicate injectable#8672
amiramw merged 1 commit intomasterfrom
CodeEditorWidgetUtil

Conversation

@amiramw
Copy link
Member

@amiramw amiramw commented Oct 27, 2020

Signed-off-by: Amiram Wingarten amiram.wingarten@sap.com

What it does

Make CodeEditorWidget predicate injectable instead of static namespace today.

This allows downstream projects to add custom editors (not text) to the predicate and support the following scenarios for those editors:

  • editor/title contribution
  • command workbench.action.closeActiveEditor
  • command workbench.action.closeOtherEditors
  • command workbench.action.closeEditorsInGroup
  • command workbench.action.closeEditorsInOtherGroups
  • command workbench.action.closeEditorsToTheLeft
  • command workbench.action.closeEditorsToTheRight
  • command workbench.action.closeAllEditors

How to test

There shouldn't be any change in current behavior. Only refactor that allow customization in downstream projects.

  1. git clone https://github.com/tomer-epstein/vscode-close-editor.git
  2. cd vscode-close-editor && npm run compile && npm run package
  3. dsploy vscode-close-editor-0.0.1.vsix to theia
  • inspect that all ❤️ icon editor/title contributions still work corrently and exist only on editor and webview widgets
  • inspect that all these commands still work correnctly and only affect editor and webview widgets. For example if you drag some pane into the center pane it is not affected by these command, just like before.

Review checklist

Reminder for reviewers

@vince-fugnitto vince-fugnitto added extensibility issues to simplify ability to extend Theia plug-in system issues related to the plug-in system labels Oct 27, 2020
This allows downstream projects to add custom editors (not text) to the predicate and support the following scenarios for those editors:
- editor/title contribution
- command workbench.action.closeActiveEditor
- command workbench.action.closeOtherEditors
- command workbench.action.closeEditorsInGroup
- command workbench.action.closeEditorsInOtherGroups
- command workbench.action.closeEditorsToTheLeft
- command workbench.action.closeEditorsToTheRight
- command workbench.action.closeAllEditors

Signed-off-by: Amiram Wingarten <amiram.wingarten@sap.com>
@amiramw amiramw force-pushed the CodeEditorWidgetUtil branch from de720df to b07e54e Compare October 27, 2020 19:10
@amiramw
Copy link
Member Author

amiramw commented Oct 28, 2020

@vince-fugnitto @vzhukovskii @azatsarynnyy is it possible for one of you to review this PR today? It would be great if we can get it in this week release.

@azatsarynnyy
Copy link
Member

Sure, I'll look at it today.

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me 👍
I verified both the code and the fact that the provided plugin works correctly for all the available heart commands for different use-cases.

is(arg: any): arg is CodeEditorWidget {
return arg instanceof EditorWidget || arg instanceof WebviewWidget;
}
export function getResourceUri(editor: CodeEditorWidget): CodeUri | undefined {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: I'm not sure we need a breaking change note for this method, but I can certainly add a note tomorrow when I prepare the changelog for the release.

@amiramw amiramw merged commit b3aa18f into master Oct 28, 2020
@amiramw amiramw deleted the CodeEditorWidgetUtil branch October 28, 2020 14:22
@github-actions github-actions bot added this to the 1.7.0 milestone Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

extensibility issues to simplify ability to extend Theia plug-in system issues related to the plug-in system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants