onDidSaveTextDocument event does not fire when saving a non-dirty file

[jomiller]

When saving a file that has no unsaved changes (i.e. TextDocument.isDirty === false), the workspace.onWillSaveTextDocument event fires, but the workspace.onDidSaveTextDocument event does not fire. However, the non-dirty file is still saved to the file system (its timestamp is updated).

I'd like the state to be consistent between events firing and files being saved to the file system. So, either fire onDidSaveTextDocument, or do not save the file to the file system.

[vscodebot]

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

• API - Event that fires when a file is opened (#66143)
• Add an API that fires after all documents are saved as part of a Save All action (#42913)

[jrieken]

This is controlled via https://github.com/Microsoft/vscode/blob/14aa41aca780a28fdd992b9b66fce6e9de2207ab/src/vs/workbench/api/electron-browser/mainThreadDocuments.ts#L104-L108

[bpasero]

Have to check if it makes sense to fire this event when the file is not dirty and what the implications are.

[bpasero]

When saving a file that is not dirty, we run through a method doTouch that actually triggers save participants etc. but does not call the StateChange.SAVED event after done which normally causes the extension host to get this event.

I see not big reason why we could not add that one line into the touch method to have extensions be notified. The only gotcha is that previously every call to vscode.workspace.onDidSaveTextDocument would provide a document.version that is different from the previous one. Now with this change it would mean you might get the document with a same version in the event because the model did not change. But I think we can tolerate that, given the documentation says: The version number of this document (it will strictly increase after each change, including undo/redo).

[jrieken]

@bpasero does the doTouch method actually change something on disk? Iff not then I would say the behaviour is correct, we fire will-save and when that doesn't yield in actual changes we don't save and therefore no did-save event happens. However, iff something on disk changes then we should consider this

[bpasero]

@jrieken yeah it actually writes the contents to disk as they are to cause the OS to advance the mtime of the file. We do that so that you can e.g. force an external watcher to retrigger whatever it is doing (e.g. buil).

[jrieken]

Ok, that's changes things and we should fire the change event when stuff actually changes on disk. cc @DanTup how will like this new insights.

[DanTup]

Indeed! :) I actually noticed this the other day - I changed my things over to use watchers (not specifically to workaround this, but so that changing branch in git could be detected too) and noticed that hitting Ctrl+S in the file with no changes still triggered it.

Even using the watcher it'll let me clean some things up, so this would be a nice change :-) (and if any handlers don't want to do work when the file wasn't actually changed, I presume they're able to check the isDirty flag themselves anyway).

@jrieken thanks for the heads-up! :)

[bpasero]

via #73123