Search editor: investigate to use file working copies

[bpasero]

In the context of #117873 I was doing a quick research how working copy providers that are text file model based are implemented today and found 2 examples:

• custom text based editors
• search editors

When you look at CustomTextEditorModel you can see how every IO related method (like save or revert) is going into the manager of text file models, e.g.:

vscode/src/vs/workbench/contrib/customEditor/common/customTextEditorModel.ts

Line 95 in 5d5b9dd

return this.textFileService.save(this.resource, options);

This has the advantage that a lot of corner cases for saving are being taken care of that prevent data loss or corruption.

When looking at search editors, I see that a (code editor) model is being resolved or created here:

vscode/src/vs/workbench/contrib/searchEditor/browser/searchEditorModel.ts

Line 61 in 7177658

const model = this.modelService.getModel(this.modelUri) ?? this.modelService.createModel(contents, this.modeService.create('search-result'), this.modelUri);

But the IO logic is all implemented custom, e.g.:

vscode/src/vs/workbench/contrib/searchEditor/browser/searchEditorInput.ts

Line 128 in 57488b5

await this.textFileService.write(this.backingUri, await this.serializeForDisk(), options);

As such, your custom search editor is a good candidate for adopting the text file model approach, similar to custom text editors.

I am inclined to mark this important because I think you really should not implement the IO logic yourself, but maybe there are no issues with your implementation either, so I leave it as debt.

[bpasero]

Ok, not good. There is an easy way to produce dirty overwrites which is something we protect the user from when it comes to text editors, steps:

• open a search editor that is saved on disk and make it dirty
• open the same search editor in a new window and modify it
• go back to the original window and just save your changes

🐛 you have essentially overwritten the changes that you made before

On top of this there is a large list of things that the search editor currently don't support which we should get it to support, from the list at #117873 (comment) for example, search editors should automatically reload when they change contents, which currently does not seem to be the case.

[devuxer]

@bpasero , Would this also solve a (much lesser critical issue) I'm having where hovering a word fails to show the definition (e.g., I do not get type info for a TS type if I hover)? I was tempted to open a feature request for this, but will not if this covers it. Thx.

[JacksonKearl]

@devuxer this would not change that behavior, you can open a new feature request for that.

[devuxer]

@JacksonKearl, Thanks. Just posted a new feature request.

[JacksonKearl]

@bpasero thanks for this report, much of this logic was written custom because the model as exists to the model service only contains the actual text inside the editor, but what we work with on disk has the search configuration serialized and prepended to that model's contents. Thus I call the textFileService.write and .create commands explicitly with the serialized text:

vscode/src/vs/workbench/contrib/searchEditor/browser/searchEditorInput.ts

Lines 148 to 149 in 5e3df4c

	const toWrite = await this.serializeForDisk();
	if (await this.textFileService.create([{ resource: path, value: toWrite, options: { overwrite: true } }])) {

vscode/src/vs/workbench/contrib/searchEditor/browser/searchEditorInput.ts

Line 128 in 5e3df4c

await this.textFileService.write(this.backingUri, await this.serializeForDisk(), options);

It's not clear to me how the .save and .saveAs commands can be made to work with this model, do you have any suggestions?

Also, for the act of updating the view when the contents on disk change, what's the normal procedure for watching for file changes? Is this something done by the view or is there a service to use?

[bpasero]

@JacksonKearl maybe you could benefit from the file working copy concept if you do not want to leverage text file models. We will polish the API during this month for notebooks, some things are not yet there. But the gist is that you get the full support of what text file models have today without actually instantiating any text models. The API is simple byte based and you only need to implement IFileWorkingCopyModel to leverage it:

vscode/src/vs/workbench/services/workingCopy/common/fileWorkingCopy.ts

Line 46 in 7f9835a

export interface IFileWorkingCopyModel extends IDisposable {

This code also takes care of updating the model when there are changes on disk. It pretty much takes over all of the aspects of saving, resolving, reverting, backups, file events, etc.

The work happens in #117873

[bpasero]

The file working copy API is now in good shape to be used.

[bpasero]

After a 1on1 with Jackson, there is some homework on my end to figure out if file working copies can also help for the untitled case and also to provide some guidance how to actually use file working copies.

Since the majority of users seem to use untitled search editors and from my testing I could not find issues with that, removing important label, since this is only about saved search editors.

I also suggest to move this to May, so that I can investigate for how to support untitled working copies if possible.

[JacksonKearl]

IMO the risk/benefit of this change is too large at this point, there have been no issues regarding the potential for desync here and migrating things over would touch quite a lot of otherwise solid search editor code.