Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Share "Reopen with" command and Editor Associations with notebooks, settings and keybindings UIs #94408

Closed
Tyriar opened this issue Apr 3, 2020 · 27 comments
Assignees
Labels
feature-request Request for new features or functionality *out-of-scope Posted issue is not in scope of VS Code polish Cleanup and polish issue workbench-actions Workbench Actions Support

Comments

@Tyriar
Copy link
Member

Tyriar commented Apr 3, 2020

Notebooks, settings UI and the keybindings UI are very similar to custom editors. We should make the command more generic so the method to view the backing file in the text editor is shared and behaves the same across each of these editors.

@rebornix
Copy link
Member

rebornix commented Apr 3, 2020

Internally we are all using EditorService.overrideOpenEditor(handler: IOpenEditorOverrideHandler): IDisposable; so it makes better sense that we implement the Reopen ... inside the service.

@rebornix rebornix added the polish Cleanup and polish issue label Apr 3, 2020
@rebornix rebornix added this to the April 2020 milestone Apr 3, 2020
@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented Apr 8, 2020

I'm thinking about prototyping a "PowerShell Notebooks" experience where the backing file is a regular .ps1.

I want to be able to toggle between rendering the .ps1 in the regular text editor & rendering the .ps1 as a Notebook.

Having a mechanism to do this would be fantastic.

@rebornix
Copy link
Member

rebornix commented Apr 9, 2020

Kapture 2020-04-08 at 17 55 03

Above is the prototype for shared "Reopen with..." command. Previously every "custom" editor registers a simple editor override handler

export interface IOpenEditorOverrideHandler {
  (editor: IEditorInput, options: IEditorOptions | ITextEditorOptions | undefined, group: IEditorGroup): IOpenEditorOverride | undefined;
}
interface IEditorService
  overrideOpenEditor(handler: IOpenEditorOverrideHandler): IDisposable;
}

Now the IOpenEditorOverrideHandler is enriched as

export interface IOpenEditorOverrideHandler {
  open: (editor: IEditorInput, options: IEditorOptions | ITextEditorOptions | undefined, group: IEditorGroup) => IOpenEditorOverride | undefined;
  canOpen: (editor: IEditorInput, options: IEditorOptions | undefined, group: IEditorGroup | undefined) => {  id?: string; label: string; description?: string; detail?: string; }[];
}

The shared "File: Reopen with..." action will show all custom editors (which can be multiple CustomEditors, multiple Notebook Editors, two types of Settings Editors and one Search Editor) which can handle the resource for users to choose from.

We also need to think about how to configure the default editor for a resource. @mjbvz added settings for CustomEditor but considering that the same file extension can be supported by different contributors.

@mjbvz @sandy081 @roblourens @JacksonKearl thoughts?

@rebornix
Copy link
Member

rebornix commented Apr 9, 2020

As the first step, I can add this action and adopt it in Notebook and then later on you can adopt it gradually in your "custom" editor.

@rebornix rebornix added the workbench-actions Workbench Actions Support label Apr 9, 2020
@rebornix
Copy link
Member

rebornix commented Apr 9, 2020

The QuickPick now aligns with Custom Editor per feedback from today's sit-down

image

Pushed my first attempt to master dc1746e . To adopt the change in your editorOverride:

1. Implement IOpenEditorOverrideHandler.getEditorOverrides

 this.editorService.overrideOpenEditor({
  getEditorOverrides: (editor: IEditorInput, options: IEditorOptions | undefined, group: IEditorGroup | undefined) => {
  ...
  return [
    {
      label,
      id,
      active,
      detail
    }
  ];
  }
}

Note that the editorInput passed in can be any kind of IEditorInput, while previously it's FileEditorInput most of the time.

The return type is IOpenEditorOverrideEntry[]. If you are returning multiple entries, make sure the id property is unique thus you know which one users pick

Handle Reopening

The editor input reopen override handler now has one more optional parameter id. If it's undefined, it means users attempt to open the file through File Explorer, etc. If it's not empty, users pick the IOpenEditorOverrideEntry explicitly.

@mjbvz
Copy link
Collaborator

mjbvz commented Apr 9, 2020

Thanks @rebornix. The approach looks good like a good start. I'll look into adopting it for custom editors.

I think it's also worth considering adopting more concepts from custom editors for notebooks and settings. For example, the workbench.editorAssociations setting lets users configure which custom editor is used for a given resource. This is very similar to what the "workbench.settings.editor" setting also does.

@rebornix
Copy link
Member

rebornix commented Apr 10, 2020

@mjbvz workbench.editorAssociations sounds a good config for mapping between file associations with "custom" editors. We use viewType as well for the notebook, in order to align with Custom Editor. So I think we can move it out to workbench/files from custom editor?

image

Settings and Keybinding can also register themselves with viewType like settings.ui, settings.json, keybinding.ui, keybindings.json and then users can use workbench.editorAssociations and deprecate settings like workbench.settings.useSplitJSON.

@rebornix rebornix changed the title Share "Reopen with" command with notebooks, settings and keybindings UIs Share "Reopen with" command and Editor Associations with notebooks, settings and keybindings UIs Apr 10, 2020
mjbvz added a commit that referenced this issue Apr 15, 2020
@mjbvz
Copy link
Collaborator

mjbvz commented Apr 15, 2020

@rebornix I hooked custom editors up to use the shared api.

The only missing feature from the shared command compared the custom editor one is that custom editors show a gear icon:

Screen Shot 2020-04-15 at 12 50 26 PM

Clicking this gear sets workbench.editorAssociations for the file. I feel this improves the discoverability of workbench.editorAssociations

I see two options:

  • Add the gear as a shared feature. Clicking on it would set workbench.editorAssociations for both notebooks and custom editors

  • Let custom editors contribute a button to the reopen with quick pick.

@rebornix Would notebooks also benefit from making workbench.editorAssociations more easily configurable?

@rebornix
Copy link
Member

Add the gear as a shared feature. Clicking on it would set workbench.editorAssociations for both notebooks and custom editors

@mjbvz yes, notebook will have similar problems and having the gear as a shared feature makes best sense. Will you do the migration? I can do that as well.

@rebornix
Copy link
Member

@mjbvz adopted the gear icon in the quick pick, which will set editor association. It's safe to deprecate the "View: Reopen with..." command from Custom Editor now.

@mjbvz
Copy link
Collaborator

mjbvz commented Apr 20, 2020

@rebornix Thanks! I'll move over any other custom editor functionality that makes sense to share

@TylerLeonhardt
Copy link
Member

@rebornix is this an editor command I can take advantage of in an extension?

@rebornix
Copy link
Member

This one is an internal one for now but I've heard that @mjbvz is making a generic public command to open a resource with a specific "custom editor"

mjbvz added a commit that referenced this issue Apr 21, 2020
For #94408

Moves this out of custom editors
@mjbvz
Copy link
Collaborator

mjbvz commented Apr 21, 2020

70a890e make the vscode.openWith command support other editor types besides custom editors

@sandy081
Copy link
Member

sandy081 commented Jul 1, 2020

Can somebody please explain to me what needs to be done here for Keyboard Shortcuts editor and any code references?

@rebornix
Copy link
Member

rebornix commented Jul 7, 2020

@sandy081 Keybindings contribution needs to register itself as an IOpenEditorOverrideHandler

overrideOpenEditor(handler: IOpenEditorOverrideHandler): IDisposable;
and as a handler, it is responsible for

  1. getEditorOverrides, provides override candidates for a specific resource. For example, there are three different view types for keybindings.json: UI, Split JSON editor, simple JSON editor. The return result will be used in Editor: Reopen With.
  2. open(editor: IEditorInput, options: IEditorOptions | ITextEditorOptions | undefined, group: IEditorGroup, context: OpenEditorContext, handles the editor opening override This one is a bit tricky and we are looking into improving it, but now it works as
  • If users try to open keybindings.json directly, editor is a FileEditorInput. If options?.override is not set, we should open Keybindings Editor based on user settings (for example, by default it opens the UI editor)
  • if options?.override is set, open the right editor based on options?.override (it's the id of the override candidate)

@sandy081 sandy081 modified the milestones: July 2020, August 2020 Jul 29, 2020
@sandy081 sandy081 modified the milestones: August 2020, September 2020 Aug 31, 2020
@mjbvz mjbvz modified the milestones: September 2020, October 2020 Sep 28, 2020
@sandy081 sandy081 modified the milestones: October 2020, November 2020 Oct 26, 2020
@mjbvz mjbvz added the feature-request Request for new features or functionality label Nov 3, 2020
@sandy081 sandy081 modified the milestones: January 2021, February 2021 Jan 27, 2021
@mjbvz mjbvz removed their assignment Feb 5, 2021
@sandy081 sandy081 modified the milestones: February 2021, March 2021 Feb 22, 2021
@sandy081 sandy081 modified the milestones: March 2021, April 2021 Mar 22, 2021
@sandy081 sandy081 modified the milestones: April 2021, May 2021 Apr 26, 2021
@sandy081 sandy081 modified the milestones: May 2021, Backlog Jun 3, 2021
@sandy081 sandy081 added the *out-of-scope Posted issue is not in scope of VS Code label Jan 16, 2024
@vscodenpa
Copy link

We closed this issue because we don't plan to address it in the foreseeable future. If you disagree and feel that this issue is crucial: we are happy to listen and to reconsider.

If you wonder what we are up to, please see our roadmap and issue reporting guidelines.

Thanks for your understanding, and happy coding!

@vscodenpa vscodenpa closed this as not planned Won't fix, can't repro, duplicate, stale Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality *out-of-scope Posted issue is not in scope of VS Code polish Cleanup and polish issue workbench-actions Workbench Actions Support
Projects
None yet
Development

No branches or pull requests

9 participants