Notebook shows dirty indicator with autosave on

[roblourens]

• Set auto save: afterDelay
• Edit a notebook
• See the dirty indicator briefly appear after editing

[roblourens]

I guess this is #96400

[bpasero]

We never did that for custom editors because it requires code to ensure that an editor that is closed but pending to be saved via auto save is really waited upon on shutdown. To further explain:

• the behaviour is driven by EditorInput#isSaving method
• if you return true from that method no dirty indicator will show up
• this means a user can close an editor with dirty changes
• as such, components have to ensure that the save continues in the background and even on shutdown

Files implement this here:

vscode/src/vs/workbench/contrib/files/browser/editors/fileEditorInput.ts

Lines 279 to 294 in 4cd3294

	override isSaving(): boolean {
	if (this.model?.hasState(TextFileEditorModelState.SAVED) || this.model?.hasState(TextFileEditorModelState.CONFLICT) || this.model?.hasState(TextFileEditorModelState.ERROR)) {
	return false; // require the model to be dirty and not in conflict or error state
	}
	// Note: currently not checking for ModelState.PENDING_SAVE for a reason
	// because we currently miss an event for this state change on editors
	// and it could result in bad UX where an editor can be closed even though
	// it shows up as dirty and has not finished saving yet.
	if (this.filesConfigurationService.getAutoSaveMode() === AutoSaveMode.AFTER_SHORT_DELAY) {
	return true; // a short auto save is configured, treat this as being saved
	}
	return super.isSaving();
	}

I think the reason we never adopted this for notebooks as that notebooks have a dual model: complex and simple. With simple (which is based on file working copies), we can probably implement this now similar to what files do, but it needs careful testing what happens when you close a notebook quickly enough before auto save finished.

[rebornix]

We could revisit this once we remove complex notebook model and friends #144810 but the saving of notebooks is more complex than files and I wonder if it's worth it to implement this. Also the dirty indicator is also an indicator that saving is not done yet, which is valuable as saving notebooks is not necessarily instant. what do you think @roblourens ?

[roblourens]

I think it's ok to close if this is complicated. It's also a bit odd that if I edit and immediately close the tab, it asks me whether I want to save, when I've enabled autosave.

[bpasero]

@amunger with your recent changes in #187304 and #187302 please take note of #140069 (comment).

Specifically: being able to close an editor that is now not dirty because of auto-save may result in data loss if closing the editor disposes state that is needed for saving.

Filed #188452 as planned for July.

Nevertheless, I appreciate that you are willing to look into this issue 🙏