Skip to content

Editor - Prompt warning when overwriting a file that is modified on disk #2783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 12, 2018
+99 −8
Merged

Editor - Prompt warning when overwriting a file that is modified on disk #2783

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9d1ba4b
added overwrite prevention to saving
unnamedplay-r Aug 19, 2017
8c90419
rearranging order of require variables and edit to rename
unnamedplay-r Aug 19, 2017
d468081
added documentation, and fixed reload button
unnamedplay-r Aug 22, 2017
4d45ec7
followed suggestion by tom, started tests
unnamedplay-r Aug 29, 2017
889f6c2
Revert "followed suggestion by tom, started tests"
unnamedplay-r Jan 4, 2018
b0dec20
added back in reverted changes to editor.js
unnamedplay-r Jan 4, 2018
7a77292
Fix broken reference to 'this'
takluyver Feb 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 99 additions & 8 deletions notebook/static/edit/js/editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
define([
'jquery',
'base/js/utils',
'base/js/i18n',
'base/js/dialog',
'codemirror/lib/codemirror',
'codemirror/mode/meta',
'codemirror/addon/comment/comment',
Expand All @@ -19,6 +21,8 @@ define([
function(
$,
utils,
i18n,
dialog,
CodeMirror
) {
"use strict";
Expand All @@ -33,6 +37,8 @@ function(
this.file_path = options.file_path;
this.config = options.config;
this.file_extension_modes = options.file_extension_modes || {};
this.last_modified = null;
this._changed_on_disk_dialog = null;

this.codemirror = new CodeMirror($(this.selector)[0]);
this.codemirror.on('changes', function(cm, changes){
Expand Down Expand Up @@ -100,6 +106,7 @@ function(
that.generation = cm.changeGeneration();
that.events.trigger("file_loaded.Editor", model);
that._clean_state();
that.last_modified = new Date(model.last_modified);
}).catch(
function(error) {
that.events.trigger("file_load_failed.Editor", error);
Expand Down Expand Up @@ -191,6 +198,11 @@ function(
}
};

/**
* Rename the file.
* @param {string} new_name
* @return {Promise} promise that resolves when the file is renamed.
*/
Editor.prototype.rename = function (new_name) {
/** rename the file */
var that = this;
Expand All @@ -200,32 +212,111 @@ function(
function (model) {
that.file_path = model.path;
that.events.trigger('file_renamed.Editor', model);
that.last_modified = new Date(model.last_modified);
that._set_mode_for_model(model);
that._clean_state();
}
);
};

Editor.prototype.save = function () {

/**
* Save this file on the server.
*
* @param {boolean} check_last_modified - checks if file has been modified on disk
* @return {Promise} - promise that resolves when the notebook is saved.
*/
Editor.prototype.save = function (check_last_modified) {
/** save the file */
if (!this.save_enabled) {
console.log("Not saving, save disabled");
return;
}

// used to check for last modified saves
if (check_last_modified === undefined) {
check_last_modified = true;
}

var model = {
path: this.file_path,
type: 'file',
format: 'text',
content: this.codemirror.getValue(),
};
var that = this;
// record change generation for isClean
this.generation = this.codemirror.changeGeneration();
that.events.trigger("file_saving.Editor");
return this.contents.save(this.file_path, model).then(function(data) {
that.events.trigger("file_saved.Editor", data);
that._clean_state();
});

var _save = function () {
that.events.trigger("file_saving.Editor");
return that.contents.save(that.file_path, model).then(function(data) {
// record change generation for isClean
that.generation = that.codemirror.changeGeneration();
that.events.trigger("file_saved.Editor", data);
that.last_modified = new Date(data.last_modified);
that._clean_state();
});
};

/*
* Gets the current working file, and checks if the file has been modified on disk. If so, it
* creates & opens a modal that issues the user a warning and prompts them to overwrite the file.
*
* If it can't get the working file, it builds a new file and saves.
*/
if (check_last_modified) {
return this.contents.get(that.file_path, {content: false}).then(
function check_if_modified(data) {
var last_modified = new Date(data.last_modified);
// We want to check last_modified (disk) > that.last_modified (our last save)
// In some cases the filesystem reports an inconsistent time,
// so we allow 0.5 seconds difference before complaining.
if ((last_modified.getTime() - that.last_modified.getTime()) > 500) { // 500 ms
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We recently merged a pull request to make this 0.5s difference configurable in the notebook - #3273 . Could you have a look at that and copy the functionality here? Thanks!

console.warn("Last saving was done on `"+that.last_modified+"`("+that._last_modified+"), "+
"while the current file seem to have been saved on `"+data.last_modified+"`");
if (that._changed_on_disk_dialog !== null) {
// since the modal's event bindings are removed when destroyed, we reinstate
// save & reload callbacks on the confirmation & reload buttons
that._changed_on_disk_dialog.find('.save-confirm-btn').click(_save);
that._changed_on_disk_dialog.find('.btn-warning').click(function () {window.location.reload()});

// redisplay existing dialog
that._changed_on_disk_dialog.modal('show');
} else {
// create new dialog
that._changed_on_disk_dialog = dialog.modal({
keyboard_manager: that.keyboard_manager,
title: i18n.msg._("File changed"),
body: i18n.msg._("The file has changed on disk since the last time we opened or saved it. "
+ "Do you want to overwrite the file on disk with the version open here, or load "
+ "the version on disk (reload the page)?"),
buttons: {
Reload: {
class: 'btn-warning',
click: function () {
window.location.reload();
}
},
Cancel: {},
Overwrite: {
class: 'btn-danger save-confirm-btn',
click: function () {
_save();
}
},
}
});
}
} else {
return _save();
}
}, function (error) {
console.log(error);
// maybe it has been deleted or renamed? Go ahead and save.
return _save();
})
} else {
return _save();
}
};

Editor.prototype._clean_state = function(){
Expand Down