Editor override: provide a way to override the editor input as part of createEditorInput

[bpasero]

Today our editor override works very late as part of the openEditor. This has a couple of issues:

• we end up calling each contributed override whenever an editor opens, even when you just click on a tab
• you end up with re-entrant openEditor calls from within the override
• the input that was originally used for the openEditor call is never disposed
• the caller of openEditor might get a very unexpected result from what was originally passed in

If we had a way to override the editor at the level of where the input is created things would be cleaner.

Proposal:

• we remove the override event and override handling from editorGroupView#openEditor altogether, instead the rule is that the EditorInput that is passed into a group for opening is the correct one
• we enrich the IEditorService with support for converting a URI into a EditorInput including custom editors, this can easily be added to the existing IEditorService#createEditorInput method
• we leverage the custom editor input factories for this purpose that are already registered to convert a URI into the EditorInput
• we allow to pass in a special override option that will bring up a picker to pick a custom editor (this essentially moves the picker logic from openEditorWith into IEditorService
• for the case of "Reopen with..." we can use the same concepts: by calling IEditorService.replaceEditors(activeEditor, { resource: activeEditor.resource, options: { override: <show picker> }) everything should just work
• we adopt this concept for all editors that are not standard: notebooks, custom editors, search editors, settings editor
• bonus: we can think about merging the fileInputFactory and customInputFactory into one so that we just have 1 way of resolving an editor input from a resource and not 2

[bpasero]

After discussion with @lramos15 I have updated the proposal. I am convinced that everything around resolving editors can happen in IEditorService and should not happen in editorGroupView.

[lramos15]

Adding notes from our sync up:

• Editor input factories should merge into one (we have a generic one for serialisation, the file editor input factory and the custom editor input factory)
• To get rid of the editor override handler, we need to preserve some of its today’s semantics:
  • declare that an editor input can only exist once across all groups (“singleton” support)
  • declare other constraints like opening the same resource with different view types in the same group is not supported?
  • preserve the logic of detecting move operations so that custom editors / notebooks do not dipsose a widget that is moved to another group (OpenEditorContext.MOVE_EDITOR)
• The editor service should own the lifecycle of all editor inputs because it asks factories to create them and also should dispose them after usage
• Bonus: we should adopt the new input factories everywhere and thus stop passing in typed EditorInput to the editor service but only { resource } inputs with associated override
• Bonus: we should revisit EditorOptions and maybe just allow a bag of properties to be send into the IEditorService without the need of typed options (would have also helped in the recent bug we found in settings editor)

[bpasero]

@lramos15 maybe this one can be closed now, I think my original concern is addressed, we only override now from within the editor service. I do not think that createEditorInput is the right location for the override.

[lramos15]

Alright, sounds good 👍