vscode: Editor model world should offer some amenities to protect against data loss / file corruption

Normal file editors, custom editors, and notebooks all implement editors for something on a file system. All implement the relatively rigid structure of editor inputs and editor models. However, they get little to nothing in return for implementing this, there is no service-like infrastructure that makes my live simpler, e.g

there is logic for sequentialising saves inside TextFileEditorModel and roughly the same code exists (was copied) for notebook editors. Custom editors tackle the same problem, but uses different code. So, instead of everyone re-inventing the wheel the “framework”, which interfaces I must implement, should take care of this for me.
there is duplicated logic for when a file is being deleted, e.g text file editors have inOrphanMode and custom editors have _inOrphaned. There is a utility method decorateFileEditorLabel to decorate the editor label to indicate orphaned editors but this should move into a common super type. Notebooks didn’t copy this yet. Again, I should be able to express to the framework that I am some kind of file editor and then deletion should be taken care of
the logic for general file watching is duplicated, e.g when a file on disk changes we reload it unless the file is dirty (text files, notebooks). The framework should do that automatically because I already tell it how to load and whether my model is dirty or not.
[ ] there is a TextFileEditorTracker that takes care of opening dirty models and reloading them when the window gains focus but this code is only used for text file models. Notebooks has NotebookFileTracker for that
there is a complex dirty write prevention code in file service validateWriteFile . A simple version was implemented in notebook but it has “false negative” bugs as remote/live-share cases are not covered. Custom editor doesn’t handle this yet it seems. Related: https://github.com/microsoft/vscode/issues/117715
text file models register listeners to gracefully finish any pending saves on shutdown, which notebooks and custom editors either do not do or currently fail to do with a high risk of data loss
properly restore backups for file working copy providers

I don’t think this list is complete but that’s my learning from the little exposure I have gotten via the notebook world. The current approach just seems wasteful: we duplicate code, we duplicate bugs, we duplicate work.

About this issue

Original URL
State: closed
Created 3 years ago
Comments: 17 (16 by maintainers)

Commits related to this issue

Extract decorateFileEditorLabel to a common function I previously duplicated this logic. It makes sense to reuse the strings and logic for consistency From #117873 — committed to microsoft/vscode by mjbvz 3 years ago
file working copy - provide some basic save error handling (#117873) — committed to microsoft/vscode by bpasero 3 years ago
working copy - first cut of a backup restorer that works on new working copy editor service (#117873) — committed to microsoft/vscode by bpasero 3 years ago
working copy - implement backup restore in tracker and keep non restored backups (#117873) — committed to microsoft/vscode by bpasero 3 years ago

Most upvoted comments

I have pushed a first version of IFileWorkingCopy and IFileWorkingCopyManager (see https://github.com/microsoft/vscode/pull/119233) which so far covers these things:

save sequentialising
dirty write prevention
conditional load (aka return early if file has not changed on disk)
orphaned file detection
reloading model on file changes (unless dirty)
undo handling (marking working copy as saved when undoing to saved version)
automated backup handling with file stat metadata (to preserve dirty write prevention across restarts)

bpasero on Mar 19, 2021

After 1on1 with Jo, here are some ideas going forward what workbench can provide: assuming that we typically have a model where a working copy is always backed by 1 file resource (either text or binary - doesn’t really matter), we can move and share a lot of the code around text file models to be reused by probably all notebook and custom editors (even search editor).

Rough ideas:

introduce an abstract working copy super type that is file resource based and provides
- sequential saving support
- dirty write prevention (via stat and etag) [1]
- maybe: conditional read support (aka don’t load the model again if etag match)
introduce the notion of file based working copies to working copy service [2]
- observes working copies that are pending to be saved to join on shutdown to prevent data corruption
- watches for changes on the file resource to
  - reload the working copy on changes
  - toggle orphaned state for deletions
- ensures dirty working copies are opened as editors
- ensures working copies are reloaded when focus moves back to the window

[1] For now, save conflict resolution would probably be simple and not involve the more complex diff support we have for text files

[2] This requires some changes to the registration of working copies: We should allow to register the multiple working copies of the same resource because that can easily happen when the same file is opened with different editors. Maybe a combination of resource + editorId / viewType could be used, because the working copy service (or some contributed tracker) will have to know how to open an editor when a working copy is getting dirty

bpasero on Mar 4, 2021

@jrieken fyi I added 2 more entries to your list: the text file editor tracker and the text file save error handler

bpasero on Mar 1, 2021