feat: add ability to create custom editors for files

[CompuIves]

This PR is a bit more tricky. I've been looking into ways that we can render custom editors for files. Our specific use case is rendering a custom interactive Markdown for markdown files, and we've been wanting to use React for this as it's more integrated with our editor.

While there's currently a nice API for creating custom editors, it does not handle creating custom editors for files yet. The missing piece to enable this is the IEditorResolverService, you can register a custom editor like so:

  const registerEditor = ({
    id,
    name,
    onRender,
    globPattern,
  }: RegisterEditorParams) => {
    const disposableStore = new DisposableStore();

    // First create our custom editor
    const { disposable, CustomEditorInput } = registerEditorPane({
      id: id,
      name,
      renderBody: (container) => {
        return onRender(container);
      },
    });
    disposableStore.add(disposable);

    // Then register that editor for the editor resolver service, to tell VSCode to use this
    // editor when the globPattern matches (e.g. `*.md`, or `csb-file:/**`).
    const editorResolverService = StandaloneServices.get(
      IEditorResolverService
    );
    const resolveDisposable = editorResolverService.registerEditor(
      globPattern,
      {
        id,
        label: name,
        // @ts-ignore
        priority: "default" as const,
      },
      {},
      {
        createEditorInput: (baseInput) => {
          return {
            options: {},
            editor: new CustomEditorInput(undefined, baseInput, id),
          };
        },
      }
    );
    disposableStore.add(resolveDisposable);

    return disposableStore;
  };

This works really well, but the last missing piece is that VSCode will call setInput on the editor to set the actual input. It does this, because it reuses editors and swaps the underlying model/input. We need a way to respond to that setInput call, and so I extended the onRender call to not just respond with a disposable, but also with an optional setInput.

[CGNonofr]

Can you please fix the eslint warning and the demo?

[CGNonofr]

a little bit complex indeed :)

What about adding the globPattern parameter to the registerEditorPane function? Do you think it makes sense?

Also can you demonstrate it in the demo?

[CompuIves]

Do you think it makes sense?

The user would lose some customisability (e.g. can't pass editor options, or define how the untitled editor should launch). But at the same time, it would make the API more accessible. I can add it as an optional parameter. If people then don't pass it they still have the opportunity to register it in a custom way themselves 😄.

[CGNonofr]

Do you think it makes sense?

The user would lose some customisability (e.g. can't pass editor options, or define how the untitled editor should launch). But at the same time, it would make the API more accessible. I can add it as an optional parameter. If people then don't pass it they still have the opportunity to register it in a custom way themselves 😄.

Yep, it looks like a good compromise

[CompuIves]

How can I get access to the editorResolverService from within the function? Is there a way that I can get access to the instantiationService?

[CGNonofr]

How can I get access to the editorResolverService from within the function? Is there a way that I can get access to the instantiationService?

StandaloneServices.get is fine. You just need to make sure the services are initialized, either by wrapping it into a StandaloneServices.withServices or using getService from services.ts

[CompuIves]

Okay, will apply the changes in about ~45min!

[CGNonofr]

Okay, will apply the changes in about ~45min!

TIC TAC TIC TAC 😄

[CompuIves]

On it! 😂

[CompuIves]

Okay, updated, but I can't get the demo running, so will need to double check that.

[CGNonofr]

Okay, updated, but I can't get the demo running, so will need to double check that.

What is the error?

[CompuIves]

This one:

When building

[CompuIves]

Got it fixed! Had to reinstall xterm-addon-canvas.

[CompuIves]

Okay, this change is not working yet. Because Monaco triggers onDidInitialize before the storage service is initialized, some services get initialized without the storage service being available. I think we'll need to also only trigger onDidInitialize after we ran the participants..

[CompuIves]

Or... No, our withServices causes the initialization to happen to soon. So as long as we don't use withServices, we can make it work.

[CGNonofr]

Not sure to follow, what is the solution?

maybe you can use getService which also wait for service initialization participants

[CompuIves]

Ah, getService sounds like the right solution. I'll try that. The problem right now is that with withServices my callback gets executed before the participants have been run, and when I instantiate the editorResolverService it cannot access the storageService yet.

[CompuIves]

Okay, confirmed that with this push everything is working!

[CGNonofr]

There is still a few comments from me you didn't address :)

[CompuIves]

Ah, hmm. So for these comments:

What about adding the globPattern parameter to the registerEditorPane function? Do you think it makes sense?

The registerEditorPane now has a globPattern argument, but it's behind a specific options object. Allows us to expand it with more customization if we want to in the future.

Also can you demonstrate it in the demo?

The existing custom editor is now also opened for .md files.

Are there some other comments that I missed?

[CGNonofr]

Yes there are

[CompuIves]

Oh they're still on "Pending"! So I can't see them yet, I'll work on them from the screenshot.

[CompuIves]

Addressed the feedback!

[CGNonofr]

Oh they're still on "Pending"! So I can't see them yet, I'll work on them from the screenshot.

Ok totally my bad... sorry, I'm more used to gitlab than github

[CompuIves]

Okay, addressed the remaining feedback

[CGNonofr]

I just tried to play with the feature, that's a nice addition, I just have a few suggestions to improve it :)

[CompuIves]

I like these suggestions. I have a call right now, will go through them in 30min and apply them one by one.

[CompuIves]

Made some changes to the API! This API is much better, I like the suggestion.

[CGNonofr]

I'm sorry to have so many concerns, but that MR is a little tricky and it impacts the public API of the library. I'd like it to be as nice as possible because it's better to not change it afterward

[CGNonofr]

I'm not really happy with extending IDisposable here

an alternative that is used a lot inside VSCode itself is to provide a DisposableStore as the last parameter

[CompuIves]

The risk I see with that is that it's easy to forget for the consumer. If they forget to handle that parameter, then it won't get disposed. Now the type system will give an error if dispose() is not implemented on the returned value, making sure that things get cleaned up when we get rid of them.

[CGNonofr]

The dispose function being mandatory in on the cons column for me 😄 most of the time, you don't need it (for instance if you render a react component using portal?)

[CompuIves]

In our React case we do need to handle dispose, because when dispose is called we need to remove the component from our list. Otherwise the component will try to render on an element that is not attached to the DOM anymore. In React we also need to run the disposers of useEffect during that moment.

[CompuIves]

As an example, in our React case I actually realised that I forgot to properly dispose when I shared the code with you, because the dispose body was empty. I've since changed the code to this:

[CompuIves]

As a last point, I think that this can feel like VSCode, with how they often use classes for UI. E.g. you could also pass this to it:

class MarkdownRenderer extends Disposable {
  constructor() { }
  
  setInput(input) {
    this.draw();
  }
  
  private draw() { /* ... */ }
}

[CompuIves]

Will put the changes in today!

[CompuIves]

Addressed the feedback and pushed it. Only thing I haven't addressed is the disposable. I would still prefer that disposing is opt-out, instead of opt-in. Otherwise we create a footgun where users can have lingering resources that never get cleaned up even though the editor gets disposed.

[CGNonofr]

There is many concerns and questions on how to create an api that is simple enough but still allows the user to do as much as possible... That equation is complicated here.

I've realized the VSCode api allows to do a lot more than what I initialy though and I vote for refactoring it to be as close as possible to the way it's used inside VSCode itself

The result can be something like #239

What do you think?

Pros:

• It allows to inject any service in the EditorPane (like the fileService)
• It allows to do a lot more of advanced stuff
• In your use-case about markdown files and custom renderer, it would allow to make your EditorInput extend AbstractResourceEditorInput or AbstractTextResourceEditorInput (it handle the tab label automatically, allows to save from it, handle capabilities...)

cons:

• It may be more verbose for simple usages (even though it's probably already an already advanced need).
• VSCode only injects services through constructors, so any subclass needs to inject all parent injections as well to give it to its parent, and it's quite a pain here to have to do that for basic usages
• it requires experimentalDecorators to be enabled

[CompuIves]

Quick check on the PR, and I love it. More power to the implementor. I will check more thoroughly in 30min.

[CompuIves]

Okay, I like the PR! It's a good approach, and from what I can see so far, it will also work for our case. I particularly like that the tab title can stay consistent.

(even though it's probably already an already advanced need).

Agree, the closer to VSCode the better. People can always build a simple wrapper around it (probably we'll do this to make it work with React properly)

it requires experimentalDecorators to be enabled

Will check this, I think that should also be fine on our side!

[CompuIves]

Yep, works here!