From 3d64d29826561c93829f39518de71836485b50e9 Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Fri, 12 Jul 2013 02:22:05 -0500 Subject: [PATCH 1/7] fix issue 4238- 'Save As on file in working set does not preserve working set order' --- src/document/DocumentCommandHandlers.js | 7 ++++--- src/document/DocumentManager.js | 19 +++++++++++++++---- src/project/FileViewController.js | 5 +++-- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index f0835532c46..9e9a8ed9316 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -313,11 +313,12 @@ define(function (require, exports, module) { /** * Opens the given file, makes it the current document, AND adds it to the working set. * @param {!{fullPath:string}} Params for FILE_OPEN command + * @param {?Number} index - insert into the working set list at this 0-based index */ function handleFileAddToWorkingSet(commandData) { return handleFileOpen(commandData).done(function (doc) { // addToWorkingSet is synchronous - DocumentManager.addToWorkingSet(doc.file); + DocumentManager.addToWorkingSet(doc.file, commandData.index); }); } @@ -616,12 +617,12 @@ define(function (require, exports, module) { .always(_configureEditorAndResolve); } else { // Working set has file selection focus // replace original file in working set with new file + var index = DocumentManager.findInWorkingSet(doc.file.fullPath); // remove old file from working set. DocumentManager.removeFromWorkingSet(doc.file); //add new file to working set FileViewController - .addToWorkingSetAndSelect(path, - FileViewController.WORKING_SET_VIEW) + .addToWorkingSetAndSelect(path, FileViewController.WORKING_SET_VIEW, index) .always(_configureEditorAndResolve); } } diff --git a/src/document/DocumentManager.js b/src/document/DocumentManager.js index 9b4c930a561..1ee32c89f6b 100644 --- a/src/document/DocumentManager.js +++ b/src/document/DocumentManager.js @@ -221,8 +221,9 @@ define(function (require, exports, module) { * Adds the given file to the end of the working set list, if it is not already in the list. * Does not change which document is currently open in the editor. Completes synchronously. * @param {!FileEntry} file + * @param {?Number} index - insert into the working set list at this 0-based index */ - function addToWorkingSet(file) { + function addToWorkingSet(file, index) { // If doc is already in working set, don't add it again if (findInWorkingSet(file.fullPath) !== -1) { return; @@ -231,7 +232,13 @@ define(function (require, exports, module) { // Add to _workingSet making sure we store a different instance from the // one in the Document. See issue #1971 for more details. file = new NativeFileSystem.FileEntry(file.fullPath); - _workingSet.push(file); + if (!index || (index === -1)) { + // If no index is specified, just add the file to the end of the working set. + _workingSet.push(file); + } else { + // If specified, insert into the working set list at this 0-based index + _workingSet.splice(index, 0, file); + } // Add to MRU order: either first or last, depending on whether it's already the current doc or not if (_currentDocument && _currentDocument.file.fullPath === file.fullPath) { @@ -244,9 +251,13 @@ define(function (require, exports, module) { _workingSetAddedOrder.unshift(file); // Dispatch event - $(exports).triggerHandler("workingSetAdd", file); + if (!index || (index === -1)) { + $(exports).triggerHandler("workingSetAdd", file); + } else { + $(exports).triggerHandler("workingSetSort"); + } } - + /** * Adds the given file list to the end of the working set list. * Does not change which document is currently open in the editor. diff --git a/src/project/FileViewController.js b/src/project/FileViewController.js index 684794ef1ef..dec2db55a7a 100644 --- a/src/project/FileViewController.js +++ b/src/project/FileViewController.js @@ -178,11 +178,12 @@ define(function (require, exports, module) { * @param {!fullPath} * @param {?String} selectIn - specify either WORING_SET_VIEW or PROJECT_MANAGER. * Default is WORING_SET_VIEW. + * @param {?Number} index - insert into the working set list at this 0-based index * @return {!$.Promise} */ - function addToWorkingSetAndSelect(fullPath, selectIn) { + function addToWorkingSetAndSelect(fullPath, selectIn, index) { var result = new $.Deferred(), - promise = CommandManager.execute(Commands.FILE_ADD_TO_WORKING_SET, {fullPath: fullPath}); + promise = CommandManager.execute(Commands.FILE_ADD_TO_WORKING_SET, {fullPath: fullPath, index: index}); // This properly handles sending the right nofications in cases where the document // is already the current one. In that case we will want to notify with From 81b947337663659f499f4bd483fa0e44d897d293 Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Fri, 12 Jul 2013 12:43:31 -0500 Subject: [PATCH 2/7] per pull request review, updating comments and correcting conditionals --- src/document/DocumentCommandHandlers.js | 3 +-- src/document/DocumentManager.js | 6 +++--- src/project/FileViewController.js | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index 9e9a8ed9316..261e35664a2 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -312,8 +312,7 @@ define(function (require, exports, module) { /** * Opens the given file, makes it the current document, AND adds it to the working set. - * @param {!{fullPath:string}} Params for FILE_OPEN command - * @param {?Number} index - insert into the working set list at this 0-based index + * @param {!{fullPath:string, index:number=}} Params for FILE_OPEN command */ function handleFileAddToWorkingSet(commandData) { return handleFileOpen(commandData).done(function (doc) { diff --git a/src/document/DocumentManager.js b/src/document/DocumentManager.js index 1ee32c89f6b..79f3ce200ef 100644 --- a/src/document/DocumentManager.js +++ b/src/document/DocumentManager.js @@ -221,7 +221,7 @@ define(function (require, exports, module) { * Adds the given file to the end of the working set list, if it is not already in the list. * Does not change which document is currently open in the editor. Completes synchronously. * @param {!FileEntry} file - * @param {?Number} index - insert into the working set list at this 0-based index + * @param {number=} index - insert into the working set list at this 0-based index */ function addToWorkingSet(file, index) { // If doc is already in working set, don't add it again @@ -232,7 +232,7 @@ define(function (require, exports, module) { // Add to _workingSet making sure we store a different instance from the // one in the Document. See issue #1971 for more details. file = new NativeFileSystem.FileEntry(file.fullPath); - if (!index || (index === -1)) { + if ((index === undefined) || (index === null) || (index === -1)) { // If no index is specified, just add the file to the end of the working set. _workingSet.push(file); } else { @@ -251,7 +251,7 @@ define(function (require, exports, module) { _workingSetAddedOrder.unshift(file); // Dispatch event - if (!index || (index === -1)) { + if ((index === undefined) || (index === null) || (index === -1)) { $(exports).triggerHandler("workingSetAdd", file); } else { $(exports).triggerHandler("workingSetSort"); diff --git a/src/project/FileViewController.js b/src/project/FileViewController.js index dec2db55a7a..c6a04ccf258 100644 --- a/src/project/FileViewController.js +++ b/src/project/FileViewController.js @@ -178,7 +178,7 @@ define(function (require, exports, module) { * @param {!fullPath} * @param {?String} selectIn - specify either WORING_SET_VIEW or PROJECT_MANAGER. * Default is WORING_SET_VIEW. - * @param {?Number} index - insert into the working set list at this 0-based index + * @param {number=} index - insert into the working set list at this 0-based index * @return {!$.Promise} */ function addToWorkingSetAndSelect(fullPath, selectIn, index) { From 91f4931b2a61e4e2bb87651c484b4f507c8e7aec Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Fri, 12 Jul 2013 12:58:40 -0500 Subject: [PATCH 3/7] per pull request review, added code to suppress the initial removal of the item from the working set, thereby reducing the flicker otherwise caused by two redraws of the working set list --- src/document/DocumentCommandHandlers.js | 2 +- src/document/DocumentManager.js | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index 261e35664a2..1c1fc7561ba 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -618,7 +618,7 @@ define(function (require, exports, module) { // replace original file in working set with new file var index = DocumentManager.findInWorkingSet(doc.file.fullPath); // remove old file from working set. - DocumentManager.removeFromWorkingSet(doc.file); + DocumentManager.removeFromWorkingSet(doc.file, true); //add new file to working set FileViewController .addToWorkingSetAndSelect(path, FileViewController.WORKING_SET_VIEW, index) diff --git a/src/document/DocumentManager.js b/src/document/DocumentManager.js index 79f3ce200ef..41676998487 100644 --- a/src/document/DocumentManager.js +++ b/src/document/DocumentManager.js @@ -298,8 +298,9 @@ define(function (require, exports, module) { * Removes the given file from the working set list, if it was in the list. Does not change * the current editor even if it's for this file. * @param {!FileEntry} file + * @param {boolean=} true to suppress redraw after removal */ - function removeFromWorkingSet(file) { + function removeFromWorkingSet(file, suppressRedraw) { // If doc isn't in working set, do nothing var index = findInWorkingSet(file.fullPath); if (index === -1) { @@ -312,7 +313,9 @@ define(function (require, exports, module) { _workingSetAddedOrder.splice(findInWorkingSet(file.fullPath, _workingSetAddedOrder), 1); // Dispatch event - $(exports).triggerHandler("workingSetRemove", file); + if ((suppressRedraw === undefined) || (suppressRedraw === null) || (suppressRedraw !== true)) { + $(exports).triggerHandler("workingSetRemove", file); + } } /** From 48121cc3c75dd88b15479a7b31c066d3ff60089c Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Fri, 12 Jul 2013 15:04:08 -0500 Subject: [PATCH 4/7] per pull request review, pushing 'suppressRedraw' flag into the WorkingSetView handler --- src/document/DocumentManager.js | 4 +--- src/project/WorkingSetView.js | 12 +++++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/document/DocumentManager.js b/src/document/DocumentManager.js index 41676998487..9f4ded2d664 100644 --- a/src/document/DocumentManager.js +++ b/src/document/DocumentManager.js @@ -313,9 +313,7 @@ define(function (require, exports, module) { _workingSetAddedOrder.splice(findInWorkingSet(file.fullPath, _workingSetAddedOrder), 1); // Dispatch event - if ((suppressRedraw === undefined) || (suppressRedraw === null) || (suppressRedraw !== true)) { - $(exports).triggerHandler("workingSetRemove", file); - } + $(exports).triggerHandler("workingSetRemove", [file, suppressRedraw]); } /** diff --git a/src/project/WorkingSetView.js b/src/project/WorkingSetView.js index 92f65949d6c..af3563b78f9 100644 --- a/src/project/WorkingSetView.js +++ b/src/project/WorkingSetView.js @@ -511,9 +511,9 @@ define(function (require, exports, module) { /** * @private - * @param {FileEntry} file + * @param {FileEntry, suppressRedraw=} file, flag to suppress redraw if true */ - function _handleFileRemoved(file) { + function _handleFileRemoved(file, suppressRedraw) { var $listItem = _findListItemFromFile(file); if ($listItem) { // Make the next file in the list show the close icon, @@ -527,7 +527,9 @@ define(function (require, exports, module) { $listItem.remove(); } - _redraw(); + if (!suppressRedraw) { + _redraw(); + } } function _handleRemoveList(removedFiles) { @@ -592,8 +594,8 @@ define(function (require, exports, module) { _handleFileListAdded(addedFiles); }); - $(DocumentManager).on("workingSetRemove", function (event, removedFile) { - _handleFileRemoved(removedFile); + $(DocumentManager).on("workingSetRemove", function (event, removedFile, suppressRedraw) { + _handleFileRemoved(removedFile, suppressRedraw); }); $(DocumentManager).on("workingSetRemoveList", function (event, removedFiles) { From 4fac36228399c0ba8e92c3b46159b2c0af2228d6 Mon Sep 17 00:00:00 2001 From: Glenn Ruehle Date: Fri, 12 Jul 2013 15:30:43 -0700 Subject: [PATCH 5/7] JSDoc cleanup. --- src/project/WorkingSetView.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/project/WorkingSetView.js b/src/project/WorkingSetView.js index 6b220f73fa2..2c5653630b6 100644 --- a/src/project/WorkingSetView.js +++ b/src/project/WorkingSetView.js @@ -511,7 +511,8 @@ define(function (require, exports, module) { /** * @private - * @param {FileEntry, boolean=} file, flag to suppress redraw if true + * @param {FileEntry} file + * @param {boolean=} supporessRedraw If true, suppress redraw */ function _handleFileRemoved(file, suppressRedraw) { var $listItem = _findListItemFromFile(file); From 879949b1abda6174358507df0dde451304e21ca9 Mon Sep 17 00:00:00 2001 From: Bryan Chin Date: Fri, 12 Jul 2013 18:07:54 -0500 Subject: [PATCH 6/7] per pull request review, conditionalizing more of the list removal code so that the working set list isn't changed at all --- src/project/WorkingSetView.js | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/project/WorkingSetView.js b/src/project/WorkingSetView.js index 6b220f73fa2..891823f9181 100644 --- a/src/project/WorkingSetView.js +++ b/src/project/WorkingSetView.js @@ -514,20 +514,20 @@ define(function (require, exports, module) { * @param {FileEntry, boolean=} file, flag to suppress redraw if true */ function _handleFileRemoved(file, suppressRedraw) { - var $listItem = _findListItemFromFile(file); - if ($listItem) { - // Make the next file in the list show the close icon, - // without having to move the mouse, if there is a next file. - var $nextListItem = $listItem.next(); - if ($nextListItem && $nextListItem.length > 0) { - var canClose = ($listItem.find(".can-close").length === 1); - var isDirty = isOpenAndDirty($nextListItem.data(_FILE_KEY)); - _updateFileStatusIcon($nextListItem, isDirty, canClose); - } - $listItem.remove(); - } - if (!suppressRedraw) { + var $listItem = _findListItemFromFile(file); + if ($listItem) { + // Make the next file in the list show the close icon, + // without having to move the mouse, if there is a next file. + var $nextListItem = $listItem.next(); + if ($nextListItem && $nextListItem.length > 0) { + var canClose = ($listItem.find(".can-close").length === 1); + var isDirty = isOpenAndDirty($nextListItem.data(_FILE_KEY)); + _updateFileStatusIcon($nextListItem, isDirty, canClose); + } + $listItem.remove(); + } + _redraw(); } } From 7274558356d5662174ef89807c086f78b2f0ec86 Mon Sep 17 00:00:00 2001 From: Glenn Ruehle Date: Fri, 12 Jul 2013 16:34:09 -0700 Subject: [PATCH 7/7] Typo --- src/project/WorkingSetView.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/project/WorkingSetView.js b/src/project/WorkingSetView.js index a5b011c45e7..2b6f90b49d4 100644 --- a/src/project/WorkingSetView.js +++ b/src/project/WorkingSetView.js @@ -512,7 +512,7 @@ define(function (require, exports, module) { /** * @private * @param {FileEntry} file - * @param {boolean=} supporessRedraw If true, suppress redraw + * @param {boolean=} suppressRedraw If true, suppress redraw */ function _handleFileRemoved(file, suppressRedraw) { if (!suppressRedraw) {