Abort saving if a file cannot be saved #16900
Conversation
|
Thanks for making a pull request to jupyterlab! |
packages/docregistry/src/context.ts
Outdated
| if (!this.canSave) { | ||
| // File cannot be saved. | ||
| return Promise.reject(); | ||
| } |
There was a problem hiding this comment.
Just for fun, I asked Claude to review this and it reverted with this suggestion:
if (!this.canSave) {
// File cannot be saved.
return Promise.reject(new Error('File cannot be saved')); // Add error message
}
Feel free to ignore Claude! Else it all LGTM.
|
Thanks @JasonWeill for looking into this 👍 The test failures appear to be related though:
|
| if (!this.canSave) { | ||
| // File cannot be saved. The "save" command is disabled in the UI, | ||
| // but if the user tries to save anyway, act as though it succeeded. | ||
| this._saveState.emit('completed'); | ||
| return Promise.resolve(); | ||
| } |
There was a problem hiding this comment.
It would be good to consider what effect it has on the logic for updating the model hash (which is needed to prevent spurious "File Changed" dialogs). This condition prevents updating contentHash as it is performed in _maybeSave method and this condition prevents ever reaching it. I hope this would not be a problem for RTC because it now uses fileChanged signal to propagate new contentHash since:
- Update contents model on file change due to save from RTC #16695
- Fix spurious "File Changed" dialog in JupyterLab 4.1+/Notebook 7.1+ jupyter-collaboration#337
I also think that it would not make the situation worse for other drives either as in that case they would not update the hash as they could not save the content.
I think that all we need to do is document the side-effects when saving and when saving is skipped, especially if we are going to emit saveState even if the save did not happen.
JasonWeill
force-pushed
the
disable-save
branch
from
88ca556
to
8a08b6d
Compare
|
It might be good to document what happens on save (what events are emitted depending on whether save is supported or not) in https://jupyterlab.readthedocs.io/en/latest/extension/documents.html |
References
Fixes jupyterlab/jupyter-collaboration#364.
Code changes
Aborts saving if the
canSaveproperty of the context isfalse. This isfalsewhen the context's model is in collaborative mode.User-facing changes
When the user presses CTRL+S or ⌘+S with Jupyter Collaboration installed, the save command does nothing, and does not cause an error dialog to appear. Without Jupyter Collaboration installed, the command works as it did before.
Backwards-incompatible changes
None.