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

Prototyping custom editors #77789

Merged
merged 25 commits into from
Sep 11, 2019
Merged

Prototyping custom editors #77789

merged 25 commits into from
Sep 11, 2019

Conversation

mjbvz
Copy link
Collaborator

@mjbvz mjbvz commented Jul 22, 2019

For #77131

Adds a prototype of custom editors contributed by extensions. This change does the following:

  • Introduces a new contribution point for the declarative parts of a custom editor
  • Adds API for registering a webview editor provider. This lets VS Code decided when to create a webview editor
  • Adds an openWith command that lets you select which editor to use to open a resource from the file explorer
  • Adds a setting that lets you say that you always want to use a custom editor for a given file extension
  • Hooks up auto opening of a custom editor when opening a file from quick open or explorer
  • Adds a new extension that contributes a custom image preview for png and jpg files (it copies over our builtin image preview but has a hotpink background)

Still needs a lot of UX work and testing. We are also going to explore a more generic "open handler" based approach for supporting custom editors

@mjbvz mjbvz self-assigned this Jul 22, 2019
@@ -8,13 +8,16 @@ import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
import { Registry } from 'vs/platform/registry/common/platform';
import { BaseEditor } from 'vs/workbench/browser/parts/editor/baseEditor';
import { IConstructorSignature0, IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { URI } from 'vs/base/common/uri';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jrieken @bpasero Please take a look the changes to the Editor registry and the EditorInput specifically.

@mjbvz mjbvz added this to the July 2019 milestone Jul 22, 2019
{});

if (pick) {
bigInput.setPreferredCustomEditor(pick.id!);
Copy link
Member

Choose a reason for hiding this comment

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

I believe inputs are lightweight and short lived and therefore this has no effect?

@mjbvz
Copy link
Collaborator Author

mjbvz commented Jul 24, 2019

@jrieken @bpasero Ran into some problems with the current design when you reload the workbench. The main issue is that the editor contributions from extension have not been processed when we first restore the active editor, so we always end up falling back to the normal text editor instead.

Just a quick overview of the current design:

  • For each extension custom editor contribution
  • We create a new custom IEditor with a unique id.
  • On a FileEditorInput, we are able to set the id of the custom editor that we prefer using

This model is nice because it allows us to treat custom editors just like our built-in text and binary editors. However the current design requires that we create the custom IEditor objects eagerly.

Here are some sketches of how I think we could get around this:

Have a single custom editor class: CustomEditor implements IEditor

  • Eagerly register a single custom editor class that will be used for all custom editors
  • Add a method on FileEditorInput that lets us mark which preferred custom editor variant we really wish to use (hexdump, image preview, ...)
  • Replace the getPreferredEditorsForResource method with an api that lets you query which custom editors you wish to use (I think this would likely be handled by a ICustomEditorService)

Eagerly activate a placeholder custom editor while restoring state

  • When we first restore an editor input that was using a custom editor, eagerly create an unresolved custom editor type
  • Inside this custom editor, later perform the real logic to resolve the custom editor to the actual implementation

@jrieken
Copy link
Member

jrieken commented Jul 25, 2019

Yeah, I believe having one custom editor implementation is enough. It's role should then be to get extensions ready. I am not sure about the changes in FileEditorInput. My feeling is that a custom editor needs a custom editor input and that we use that as signal to open a custom editor, e.g. new CustomEditorInput(uri, 'hexdump'). That would also allow to open a file and a custom editor side by side, e.g. create a SideBySideEditorInput with a FileEditorInput and a CustomEditorInput - I am not sure how/if that would work with todays stateful input

@mjbvz
Copy link
Collaborator Author

mjbvz commented Jul 25, 2019

Yeah may seems to be what we'll have to do. The reason I tried to avoid creating a new editor input type is that i wasn't sure how to tell VS Code to instantiate our custom input type instead of FileEditorInput when you click on a resource in the explorer. I think registerFileInputFactory may help with this

@jrieken
Copy link
Member

jrieken commented Jul 29, 2019

From an API point of view we should also explore a more loose approach, e.g instead of the relying on the registerWebviewEditor function being call we could have something that registers some open handler (similar to a command) which can then decide what it does. Samples

  • a open handler registers for md-files and creates/shows a webview panel (as it is today)
  • a open handler registers for class-files and opens a text editor showing a (virtual) text document for the file

@jrieken jrieken modified the milestones: July 2019, August 2019 Jul 29, 2019
@bpasero bpasero self-requested a review August 5, 2019 08:57
Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

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

@mjbvz I would suggest to go down the path of introducing a new editor input for this work. That saves you the trouble of restoring state between restarts and would not require any changes to the FileEditorInput at all, which imho is not the right place to make these changes to begin with.

We already have a mechanism in place that allows to prevent the default action of opening an editor to replace with another editor to open by listening to onWillOpenEditor and calling the prevent method by returning a custom editor. I suggest to experiment with that approach and see how far you get. We initially added this event for the scenario of a user clicking on settings.json in the .vscode folder and then opening the settings UI editor instead of the file (we no longer seem to do that though @sandy081 @roblourens fyi). But the event is still there and can be used. We can enrich it with more stuff as needed for this feature.

Another feedback I have is: we should not tie this to webview editors alone. The binary editor extension for example is NOT using a webview editor as far as I can tell, but a normal text editor with a document that can be changed.

@tiagobento
Copy link

Hi @mjbvz! That's a really nice idea. Like the progress you made so far. Do you have plans to extend the WebviewEditor interface to implement more than the WebviewPanel interface?

I'm asking that because from the GIF you posted, I can't tell if there's editing capabilities on these "custom editors". For example, the TextEditor interface has a isDirty method, which would be nice to have on a custom editor as well.

From your TODO list, I'm assuming it is possible to edit the contents on a WebviewEditor, but then, if you edit the content of a file using a custom editor, is there a way to "override" the save command? Because I assume you'll need to convert whatever you have on your custom editor to text/bytes, right?

Anyway, sorry to appear out of nowhere, I'm just really interested on this feature :)

Thanks!

@mjbvz mjbvz force-pushed the dev/mjbvz/custom-editors branch 3 times, most recently from 1c1904c to 0fe7f6a Compare August 16, 2019 22:11
@mjbvz
Copy link
Collaborator Author

mjbvz commented Aug 16, 2019

@tiagobento See #77131

@mjbvz mjbvz force-pushed the dev/mjbvz/custom-editors branch 3 times, most recently from 8052756 to aeb38de Compare August 16, 2019 22:32
@mjbvz mjbvz force-pushed the dev/mjbvz/custom-editors branch 2 times, most recently from 90775ff to 7ab1882 Compare August 24, 2019 03:30
@alexdima alexdima modified the milestones: August 2019, September 2019 Aug 27, 2019
@mjbvz mjbvz force-pushed the dev/mjbvz/custom-editors branch from cd90d0b to a47af87 Compare September 10, 2019 22:45
For #77131

Adds a prototype of custom editors contributed by extensions. This change does the following:

- Introduces a new contribution point for the declarative parts of a custom editor
- Adds API for registering a webview editor provider. This lets VS Code decided when to create a webview editor
- Adds an `openWith` command that lets you select which editor to use to open a resource from the file explorer
- Adds a setting that lets you say that you always want to use a custom editor for a given file extension
- Hooks up auto opening of a custom editor when opening a file from quick open or explorer
- Adds a new extension that contributes a custom image preview for png and jpg files

Still needs a lot of UX work and testing. We are also going to explore a more generic "open handler" based approach for supporting custom editors

Revert
Changes `enableByDefault` boolean to a `discretion` enum. This should give more flexibility if we want other options (such as forcing a given custom editor to always be used even if there are other default ones)
@mjbvz mjbvz force-pushed the dev/mjbvz/custom-editors branch from 5ee044b to d09659b Compare September 11, 2019 00:42
@mjbvz mjbvz merged commit 011836a into master Sep 11, 2019
@mjbvz mjbvz deleted the dev/mjbvz/custom-editors branch September 11, 2019 00:56
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants