Skip to content

Commit

Permalink
Merge pull request adobe#6694 from adobe/nj/issue-6525
Browse files Browse the repository at this point in the history 
Avoid showing the keep changes/overwrite dialogs when unnecessary (for adobe#6525)
  • Loading branch information
@peterflynn
peterflynn committed Feb 4, 2014
2 parents bdd773d + 331e9b2 commit eed020c
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 21 deletions.
25 changes: 22 additions & 3 deletions src/document/Document.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,16 @@ define(function (require, exports, module) {
*/
Document.prototype.diskTimestamp = null;

/**
* The timestamp of the document at the point where the user last said to keep changes that conflict
* with the current disk version. Can also be -1, indicating that the file was deleted on disk at the
* last point when the user said to keep changes, or null, indicating that the user has not said to
* keep changes.
* Note that this is a time as returned by Date.getTime(), not a Date object.
* @type {?Number}
*/
Document.prototype.keepChangesTime = null;

/**
* True while refreshText() is in progress and change notifications shouldn't trip the dirty flag.
* @type {boolean}
Expand Down Expand Up @@ -287,8 +297,8 @@ define(function (require, exports, module) {
// either be an array of lines or a single string?
$(this).triggerHandler("change", [this, {text: text.split(/\r?\n/)}]);
}
this.diskTimestamp = newTimestamp;
this._updateTimestamp(newTimestamp);

// If Doc was dirty before refresh, reset it to clean now (don't always call, to avoid no-op dirtyFlagChange events) Since
// _resetText() above already ensures Editor state is clean, it's safe to skip _markClean() as long as our own state is already clean too.
if (this.isDirty) {
Expand Down Expand Up @@ -407,6 +417,15 @@ define(function (require, exports, module) {
$(exports).triggerHandler("_dirtyFlagChange", this);
};

/**
* @private
*/
Document.prototype._updateTimestamp = function (timestamp) {
this.diskTimestamp = timestamp;
// Clear the "keep changes" timestamp since it's no longer relevant.
this.keepChangesTime = null;
};

/**
* Called when the document is saved (which currently happens in DocumentCommandHandlers). Marks the
* document not dirty and notifies listeners of the save.
Expand All @@ -422,7 +441,7 @@ define(function (require, exports, module) {
var thisDoc = this;
this.file.stat(function (err, stat) {
if (!err) {
thisDoc.diskTimestamp = stat.mtime;
thisDoc._updateTimestamp(stat.mtime);
} else {
console.log("Error updating timestamp after saving file: " + thisDoc.file.fullPath);
}
Expand Down
26 changes: 23 additions & 3 deletions src/document/DocumentCommandHandlers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -614,9 +614,7 @@ define(function (require, exports, module) {
});
}

if (docToSave.isDirty) {
var writeError = false;

function trySave() {
// We don't want normalized line endings, so it's important to pass true to getText()
FileUtils.writeText(file, docToSave.getText(true), force)
.done(function () {
Expand All @@ -630,6 +628,28 @@ define(function (require, exports, module) {
handleError(err);
}
});
}

if (docToSave.isDirty) {
var writeError = false;

if (docToSave.keepChangesTime) {
// The user has decided to keep conflicting changes in the editor. Check to make sure
// the file hasn't changed since they last decided to do that.
docToSave.file.stat(function (err, stat) {
// If the file has been deleted on disk, the stat will return an error, but that's fine since
// that means there's no file to overwrite anyway, so the save will succeed without us having
// to set force = true.
if (!err && docToSave.keepChangesTime === stat.mtime.getTime()) {
// OK, it's safe to overwrite the file even though we never reloaded the latest version,
// since the user already said s/he wanted to ignore the disk version.
force = true;
}
trySave();
});
} else {
trySave();
}
} else {
result.resolve(file);
}
Expand Down
61 changes: 46 additions & 15 deletions src/project/FileSyncManager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ define(function (require, exports, module) {
var toReload;
/** @type {Array.<Document>} */
var toClose;
/** @type {Array.<Document>} */
/** @type {Array.<{doc: Document, fileTime: number}>} */
var editConflicts;
/** @type {Array.<Document>} */
/** @type {Array.<{doc: Document, fileTime: number}>} */
var deleteConflicts;


Expand Down Expand Up @@ -107,21 +107,41 @@ define(function (require, exports, module) {
doc.file.stat(function (err, stat) {
if (!err) {
// Does file's timestamp differ from last sync time on the Document?
if (stat.mtime.getTime() !== doc.diskTimestamp.getTime()) {
if (doc.isDirty) {
editConflicts.push(doc);
} else {
toReload.push(doc);
var fileTime = stat.mtime.getTime();
if (fileTime !== doc.diskTimestamp.getTime()) {
// If the user has chosen to keep changes that conflict with the
// current state of the file on disk, then do nothing. This means
// that even if the user later undoes back to clean, we won't
// automatically reload the file on window reactivation. We could
// make it do that, but it seems better to be consistent with the
// deletion case below, where it seems clear that you don't want
// to auto-delete the file on window reactivation just because you
// undid back to clean.
if (doc.keepChangesTime !== fileTime) {
if (doc.isDirty) {
editConflicts.push({doc: doc, fileTime: fileTime});
} else {
toReload.push(doc);
}
}
}
result.resolve();
} else {
// File has been deleted externally
if (err === FileSystemError.NOT_FOUND) {
if (doc.isDirty) {
deleteConflicts.push(doc);
} else {
toClose.push(doc);
// If the user has chosen to keep changes previously, and the file
// has been deleted, then do nothing. Like the case above, this
// means that even if the user later undoes back to clean, we won't
// then automatically delete the file on window reactivation.
// (We use -1 as the "mod time" to indicate that the file didn't
// exist, since there's no actual modification time to keep track of
// and -1 isn't a valid mod time for a real file.)
if (doc.keepChangesTime !== -1) {
if (doc.isDirty) {
deleteConflicts.push({doc: doc, fileTime: -1});
} else {
toClose.push(doc);
}
}
result.resolve();
} else {
Expand Down Expand Up @@ -251,8 +271,11 @@ define(function (require, exports, module) {

var allConflicts = editConflicts.concat(deleteConflicts);

function presentConflict(doc, i) {
var result = new $.Deferred(), promise = result.promise();
function presentConflict(docInfo, i) {
var result = new $.Deferred(),
promise = result.promise(),
doc = docInfo.doc,
fileTime = docInfo.fileTime;

// If window has been re-focused, skip all remaining conflicts so the sync can bail & restart
if (_restartPending) {
Expand Down Expand Up @@ -335,10 +358,18 @@ define(function (require, exports, module) {
}

} else {
// Cancel - if user doesn't manually save or close, we'll prompt again next
// time window is reactivated;
// Cancel - if user doesn't manually save or close, remember that they
// chose to keep the changes in the editor and don't prompt again unless the
// file changes again
// OR programmatically canceled due to _resetPending - we'll skip all
// remaining files in the conflicts list (see above)

// If this wasn't programmatically cancelled, remember that the user
// has accepted conflicting changes as of this file version.
if (!_restartPending) {
doc.keepChangesTime = fileTime;
}

result.resolve();
}
});
Expand Down

0 comments on commit eed020c

Please sign in to comment.