Continue further adoption of ITextModelResolverService

[joaomoreno]

We should get rid of StringEditorInput in favour of the ResourceEditorInput, such that we benefit from the reference counting. There is also files and untitled which are totally bypassing resolver service

1. Adopt resource URIs for all inputs in our Code

• Debug
• Output
• Git
• Keybindings
• DiffEditorInput
• SaveErrorHandler
• FileEditorInput
• UntitledEditorInput

2. Make all code (mainly workbench Editors) go through the ITextModelResolverService in one of two ways:

• Have Input.resolve return IReference<EditorModel>
• Replace input.resolve calls with ITextModelResolverService.createReference calls

[bpasero]

/cc @isidorn for adopting resource content provider in debug land. output might be a bit more tricky because we have explicit methods to append and trim values. not sure how much work it would be for debug.

[bpasero]

/cc @sandy081 to adopt resource input also for default keybindings and avoid string editor input (https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/preferences/browser/preferencesService.ts#L141)

[isidorn]

In debug I am extending StringEditorInput and using it to display memory source code. Namely when I create my input I already have all the content that I just recieved from the debug adapter. As far as I see the ResourceEditorInput makes sense to be used only when an underlying resource is on disk and when it's content should be fetched lazily. This does not seem to be the case in debug.

[isidorn]

For output we are in similar position, namely output is managing the value of the OutputEditorInput with all the append, trim methods. We could move all those methods from StringEditorInput to OutputEditorInput but I am not sure what would be the benefit of using ResourceEditorInput since each OutputEditorInput does not have a resource uri attached to it - we would have to create a fake one.

Let me know if I am missing something here

[joaomoreno]

@isidorn We want to make the ResourceEditorInput the only input available. It makes sense to be used at all times, not just in lazy scenarios. We want to reduce the internal API surface to be only resource specific, so all you have to implement is the text content provider. No need to subclass anything.

The benefit is to simplify the whole input/resource infrastructure.

[bpasero]

@isidorn I think output already has a model where the output content is in memory and it should be easy to map that to resources. Debug is more tricky: do you have a way to ask the debug adapter for the contents of the debug resource? if so, I would expect it to work like that: when the content provider is asked to return content it asks the debug adapter for it.

[isidorn]

Yes, there is a way for me to ask the deubg adapter for the content of the debug resource.
I am supposed to implement my own ITextModelResolverService to pass in to my debug input, right? Then my text model resolver service could talk to the debug extension and reslove the input when needed

[joaomoreno]

@isidorn You're supposed to implement a ITextModelContentProvider, not a ITextModelResolverService. Your provider talks to the debug extension.

Check out gitContentProvider.ts for a good reference. 👍

[isidorn]

@joaomoreno thanks for the pointer! So now when I go to the editor service and say open this uri resource the editor service will automatically get content from my content provider based on uri schema (of course after I properly register it)?

Also should I be in charge of disposing all of the ResourceEditorInput I create - right?
And is there a default content provider (could not find it) which will just read the content from disk if my ResourceEditorInput has a file uri.

[isidorn]

My motivation is to get rid of this ugly method here.

Namely I would just go to the editorService and say open this ResourceEditorInput with these options. The EditorService would find the appropriate content provider to fill the content (if in memory would use my DebugContentProvider) otherwise would use the default content provider that goes to disk.

An additional complication is that I use a special input in an error case.

Just writing my thoughts down - will try to connect the dots tomorrow morning.

[bpasero]

@joaomoreno thanks for the pointer! So now when I go to the editor service and say open this uri resource the editor service will automatically get content from my content provider based on uri schema (of course after I properly register it)?

Yes that is how it works, all you care about is resource URIs and how they resolve to content.

Also should I be in charge of disposing all of the ResourceEditorInput I create - right?
And is there a default content provider (could not find it) which will just read the content from disk if my ResourceEditorInput has a file uri.

No, since you are not dealing with inputs anymore, you do NOT dispose. The editor needs to take care of that.

Namely I would just go to the editorService and say open this ResourceEditorInput with these options. The EditorService would find the appropriate content provider to fill the content (if in memory would use my DebugContentProvider) otherwise would use the default content provider that goes to disk.

Keep in mind: no more editor inputs. Btw there is a flag called revealIfVisible that might do what you want.

[isidorn]

Phew I feel freshly showered!

Ok things that need to be done:

• @bpasero editor service should call revealLineInCenterIfOutsideViewport - otherwise there is too much jumping in my use case. E.g user is on line 100 and steps over and goes to line 101 -> the whole editor scrolls. This is bad - the viewport needs to be preserved
• @isidorn handle the error case nicer - first design how the optimal error experience should look like

Since I am out on thursday / friday if something seems badly broken just revert my commit linked here.

[bpasero]

@isidorn how did this work before, did you call this method? we cannot just "call revealLineInCenterIfOutsideViewport" everytime you open an editor, so it can only be a new option that you can pass over (similar how you can pass over line/column selection when opening an editor).

[sandy081]

Updated replace content provider to handle model updates. Also removed editor input caches since content provider takes care of updating and disposing the model. Content provider disposes the model directly until we have a new and proper dispose strategy.

[bpasero]

@sandy081 there is still the keybindings editor using StringEditorInput which we should get rid of.

[sandy081]

Ah forgot about that.

[sandy081]

Done

[joaomoreno]

@bpasero StringEditorInput is now dead. What is left?

[bpasero]

@joaomoreno adopting it for untitled editor input is something I still wanted to do eventually. It is the only one not using text model references yet.

[bpasero]

Not going to happen.