From eaa593baf0d922ffc6445d7d034c81149b8476fc Mon Sep 17 00:00:00 2001 From: sathyamoorthi Date: Sat, 12 Oct 2013 10:28:04 +0530 Subject: [PATCH 1/8] 'clearCurrentDoc' flag added to '_doCloseDocumentList' --- src/document/DocumentCommandHandlers.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index 9330f3b856f..e6b54e35c6e 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -1002,7 +1002,7 @@ define(function (require, exports, module) { // guarantees that handlers run in the order they are added. result.done(function () { if (!promptOnly) { - DocumentManager.removeListFromWorkingSet(list, (clearCurrentDoc || true)); + DocumentManager.removeListFromWorkingSet(list, clearCurrentDoc); } }); @@ -1018,11 +1018,11 @@ define(function (require, exports, module) { * @return {$.Promise} a promise that is resolved when all files are closed */ function handleFileCloseAll(commandData) { - return _doCloseDocumentList(DocumentManager.getWorkingSet(), (commandData && commandData.promptOnly)); + return _doCloseDocumentList(DocumentManager.getWorkingSet(), (commandData && commandData.promptOnly), true); } function handleFileCloseList(commandData) { - return _doCloseDocumentList((commandData && commandData.documentList), false); + return _doCloseDocumentList((commandData && commandData.documentList), false, false); } /** From 7ff5efaa617d7d3a6504307b3ceb5defb9446a34 Mon Sep 17 00:00:00 2001 From: sathyamoorthi Date: Sat, 12 Oct 2013 10:54:00 +0530 Subject: [PATCH 2/8] unit test file update --- src/extensions/default/CloseOthers/unittests.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/extensions/default/CloseOthers/unittests.js b/src/extensions/default/CloseOthers/unittests.js index 452388ea905..bb3b6e0c37a 100644 --- a/src/extensions/default/CloseOthers/unittests.js +++ b/src/extensions/default/CloseOthers/unittests.js @@ -136,6 +136,10 @@ define(function (require, exports, module) { promise = CommandManager.execute(cmdToRun); waitsForDone(promise, cmdToRun); } + + runs(function () { + expect(DocumentManager.getCurrentDocument()).not.toBe(null); + }); } it("Close others", function () { From 913cabcc5a58f1520298ee7c3f3c0bc19accb40f Mon Sep 17 00:00:00 2001 From: sathyamoorthi Date: Thu, 17 Oct 2013 11:49:10 +0530 Subject: [PATCH 3/8] Add / remove context menus at run time. --- src/extensions/default/CloseOthers/main.js | 98 ++++++++++++++----- .../default/CloseOthers/settings.json | 5 - .../default/CloseOthers/unittests.js | 4 + 3 files changed, 75 insertions(+), 32 deletions(-) delete mode 100644 src/extensions/default/CloseOthers/settings.json diff --git a/src/extensions/default/CloseOthers/main.js b/src/extensions/default/CloseOthers/main.js index a569be514f3..e6336c34621 100644 --- a/src/extensions/default/CloseOthers/main.js +++ b/src/extensions/default/CloseOthers/main.js @@ -33,22 +33,24 @@ define(function (require, exports, module) { dm = brackets.getModule("document/DocumentManager"), docCH = brackets.getModule("document/DocumentCommandHandlers"), strings = brackets.getModule("i18n!nls/strings"), - settings = JSON.parse(require("text!settings.json")), - working_set_cmenu = Menus.getContextMenu(Menus.ContextMenuIds.WORKING_SET_MENU), + workingSetCmenu = Menus.getContextMenu(Menus.ContextMenuIds.WORKING_SET_MENU), close_others = "file.close_others", close_above = "file.close_above", close_below = "file.close_below"; - - function handleClose(mode) { + + /* + This function collects files based on passing command (close_others/above/below) and execute 'FILE_CLOSE_LIST'. + */ + function handleClose(cmd) { var targetIndex = dm.findInWorkingSet(dm.getCurrentDocument().file.fullPath), workingSet = dm.getWorkingSet().slice(0), - start = (mode === close_below) ? (targetIndex + 1) : 0, - end = (mode === close_above) ? (targetIndex) : (workingSet.length), + start = (cmd === close_below) ? (targetIndex + 1) : 0, + end = (cmd === close_above) ? (targetIndex) : (workingSet.length), docList = [], i; - if (mode === close_others) { + if (cmd === close_others) { end--; workingSet.splice(targetIndex, 1); } @@ -59,25 +61,67 @@ define(function (require, exports, module) { CommandManager.execute(Commands.FILE_CLOSE_LIST, {documentList: docList}); } - - if (settings.close_below) { - CommandManager.register(strings.CMD_FILE_CLOSE_BELOW, close_below, function () { - handleClose(close_below); - }); - working_set_cmenu.addMenuItem(close_below, "", Menus.AFTER, Commands.FILE_CLOSE); - } - - if (settings.close_others) { - CommandManager.register(strings.CMD_FILE_CLOSE_OTHERS, close_others, function () { - handleClose(close_others); - }); - working_set_cmenu.addMenuItem(close_others, "", Menus.AFTER, Commands.FILE_CLOSE); - } - - if (settings.close_above) { - CommandManager.register(strings.CMD_FILE_CLOSE_ABOVE, close_above, function () { - handleClose(close_above); - }); - working_set_cmenu.addMenuItem(close_above, "", Menus.AFTER, Commands.FILE_CLOSE); + + /* + Pass 'command id' and it'll let you know, whether contextmenu for that command is existing or not. + */ + function isMenuThere(cmd) { + return Menus.getMenuItem(workingSetCmenu.id + "-" + cmd) ? true : false; } + + /* + This function is responsible for add/remove context menus based on current file selection. + If there is only one file in working set, we won't show any of the three (Close Others, Close Others Above/Below). + If there is more than one file, but selected file is first / last in working set, we will show only "Close Others". + In other cases we will show all three. + */ + $(workingSetCmenu).on("beforeContextMenuOpen", function () { + var targetIndex = dm.findInWorkingSet(dm.getCurrentDocument().file.fullPath), + closeOthers = (dm.getWorkingSet().length > 1), + closeOtherAbove = (targetIndex > 0), + closeOthersBelow = (targetIndex < dm.getWorkingSet().length - 1); + + if (closeOtherAbove && closeOthersBelow) { + if (!isMenuThere(close_above)) { + + CommandManager.register(strings.CMD_FILE_CLOSE_ABOVE, close_above, function () { + handleClose(close_above); + }); + workingSetCmenu.addMenuItem(close_above, "", Menus.AFTER, Commands.FILE_CLOSE); + } + + if (!isMenuThere(close_below)) { + CommandManager.register(strings.CMD_FILE_CLOSE_BELOW, close_below, function () { + handleClose(close_below); + }); + workingSetCmenu.addMenuItem(close_below, "", Menus.BEFORE, Commands.FILE_SAVE); + } + } else { + if (isMenuThere(close_above)) { + workingSetCmenu.removeMenuItem(close_above); + } + + if (isMenuThere(close_below)) { + workingSetCmenu.removeMenuItem(close_below); + } + } + + if (closeOthers) { + if (!isMenuThere(close_others)) { + CommandManager.register(strings.CMD_FILE_CLOSE_OTHERS, close_others, function () { + handleClose(close_others); + }); + + if (isMenuThere(close_above)) { //if "Close Others Above" exists add "Close Others" next to it + workingSetCmenu.addMenuItem(close_others, "", Menus.AFTER, close_above); + } else { //else add "Close Others" next to "Close" + workingSetCmenu.addMenuItem(close_others, "", Menus.AFTER, Commands.FILE_CLOSE); + } + } + } else { + if (isMenuThere(close_others)) { + workingSetCmenu.removeMenuItem(close_others); + } + } + }); }); \ No newline at end of file diff --git a/src/extensions/default/CloseOthers/settings.json b/src/extensions/default/CloseOthers/settings.json deleted file mode 100644 index d2250e6e2a6..00000000000 --- a/src/extensions/default/CloseOthers/settings.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "close_others": true, - "close_above": true, - "close_below": true -} \ No newline at end of file diff --git a/src/extensions/default/CloseOthers/unittests.js b/src/extensions/default/CloseOthers/unittests.js index bb3b6e0c37a..3aaae71759b 100644 --- a/src/extensions/default/CloseOthers/unittests.js +++ b/src/extensions/default/CloseOthers/unittests.js @@ -126,11 +126,15 @@ define(function (require, exports, module) { function runCloseOthers() { var ws = DocumentManager.getWorkingSet(), + e = new jQuery.Event("contextmenu"), promise; if (ws.length > docSelectIndex) { DocumentManager.getDocumentForPath(ws[docSelectIndex].fullPath).done(function (doc) { DocumentManager.setCurrentDocument(doc); + + e.pageX = 20; e.pageY = 20; + $("#open-files-container").trigger(e); }); promise = CommandManager.execute(cmdToRun); From 05e41efae9f5a6430f7c31c644ffb3d0f4d50f87 Mon Sep 17 00:00:00 2001 From: sathyamoorthi Date: Thu, 17 Oct 2013 11:55:25 +0530 Subject: [PATCH 4/8] jsHint fixes. --- src/extensions/default/CloseOthers/unittests.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/extensions/default/CloseOthers/unittests.js b/src/extensions/default/CloseOthers/unittests.js index 3aaae71759b..e288cc5ade6 100644 --- a/src/extensions/default/CloseOthers/unittests.js +++ b/src/extensions/default/CloseOthers/unittests.js @@ -22,7 +22,7 @@ */ /*jslint vars: true, plusplus: true, devel: true, browser: true, nomen: true, indent: 4, maxerr: 50 */ -/*global define, describe, it, expect, beforeEach, afterEach, runs, brackets, waitsForDone, spyOn */ +/*global define, describe, it, expect, beforeEach, afterEach, runs, brackets, waitsForDone, spyOn, jQuery */ define(function (require, exports, module) { "use strict"; @@ -133,7 +133,8 @@ define(function (require, exports, module) { DocumentManager.getDocumentForPath(ws[docSelectIndex].fullPath).done(function (doc) { DocumentManager.setCurrentDocument(doc); - e.pageX = 20; e.pageY = 20; + e.pageX = 20; + e.pageY = 20; $("#open-files-container").trigger(e); }); From d0fa2dbf133093852c6d31615ed3fe24b10c7c83 Mon Sep 17 00:00:00 2001 From: sathyamoorthi Date: Fri, 18 Oct 2013 14:49:10 +0530 Subject: [PATCH 5/8] replace `jQuery` with `$` for coding standard. --- src/extensions/default/CloseOthers/unittests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/extensions/default/CloseOthers/unittests.js b/src/extensions/default/CloseOthers/unittests.js index e288cc5ade6..c1c619c69e2 100644 --- a/src/extensions/default/CloseOthers/unittests.js +++ b/src/extensions/default/CloseOthers/unittests.js @@ -22,7 +22,7 @@ */ /*jslint vars: true, plusplus: true, devel: true, browser: true, nomen: true, indent: 4, maxerr: 50 */ -/*global define, describe, it, expect, beforeEach, afterEach, runs, brackets, waitsForDone, spyOn, jQuery */ +/*global define, describe, it, expect, beforeEach, afterEach, runs, brackets, waitsForDone, spyOn */ define(function (require, exports, module) { "use strict"; @@ -126,7 +126,7 @@ define(function (require, exports, module) { function runCloseOthers() { var ws = DocumentManager.getWorkingSet(), - e = new jQuery.Event("contextmenu"), + e = new $.Event("contextmenu"), promise; if (ws.length > docSelectIndex) { From 83d603c1a0f1bffb52a705cf741b6610bb030a7a Mon Sep 17 00:00:00 2001 From: sathyamoorthi Date: Tue, 22 Oct 2013 11:59:28 +0530 Subject: [PATCH 6/8] Code changes proposed by Jeff and Ian --- src/document/DocumentCommandHandlers.js | 15 ++++++++++----- src/extensions/default/CloseOthers/main.js | 4 ++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index e6b54e35c6e..772bb81c601 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -718,7 +718,9 @@ define(function (require, exports, module) { function _saveFileList(fileList) { // Do in serial because doSave shows error UI for each file, and we don't want to stack // multiple dialogs on top of each other - var userCanceled = false; + var userCanceled = false, + savedFiles = []; + return Async.doSequentially( fileList, function (file) { @@ -732,8 +734,7 @@ define(function (require, exports, module) { var savePromise = handleFileSave({doc: doc}); savePromise .done(function (newFile) { - file.fullPath = newFile.fullPath; - file.name = newFile.name; + savedFiles.push(newFile); }) .fail(function (error) { if (error === USER_CANCELED) { @@ -747,7 +748,9 @@ define(function (require, exports, module) { } }, false - ); + ).then(function () { + return savedFiles; + }); } /** @@ -985,7 +988,9 @@ define(function (require, exports, module) { result.reject(); } else if (id === Dialogs.DIALOG_BTN_OK) { // Save all unsaved files, then if that succeeds, close all - _saveFileList(list).done(function () { + _saveFileList(list).done(function (savedFiles) { + //saved files 'filePath' and 'fileName' will differ, if user change file name in "File Save" dialogue. + list = savedFiles; result.resolve(); }).fail(function () { result.reject(); diff --git a/src/extensions/default/CloseOthers/main.js b/src/extensions/default/CloseOthers/main.js index e6336c34621..ffead36d15e 100644 --- a/src/extensions/default/CloseOthers/main.js +++ b/src/extensions/default/CloseOthers/main.js @@ -78,10 +78,10 @@ define(function (require, exports, module) { $(workingSetCmenu).on("beforeContextMenuOpen", function () { var targetIndex = dm.findInWorkingSet(dm.getCurrentDocument().file.fullPath), closeOthers = (dm.getWorkingSet().length > 1), - closeOtherAbove = (targetIndex > 0), + closeOthersAbove = (targetIndex > 0), closeOthersBelow = (targetIndex < dm.getWorkingSet().length - 1); - if (closeOtherAbove && closeOthersBelow) { + if (closeOthersAbove && closeOthersBelow) { if (!isMenuThere(close_above)) { CommandManager.register(strings.CMD_FILE_CLOSE_ABOVE, close_above, function () { From e1efd6d712539e6e3c7d3920965b2f601301a8be Mon Sep 17 00:00:00 2001 From: sathyamoorthi Date: Wed, 23 Oct 2013 10:14:23 +0530 Subject: [PATCH 7/8] Tab alignments corrected. --- src/extensions/default/CloseOthers/unittests.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/extensions/default/CloseOthers/unittests.js b/src/extensions/default/CloseOthers/unittests.js index c1c619c69e2..ad5179a626e 100644 --- a/src/extensions/default/CloseOthers/unittests.js +++ b/src/extensions/default/CloseOthers/unittests.js @@ -123,7 +123,6 @@ define(function (require, exports, module) { SpecRunnerUtils.closeTestWindow(); }); - function runCloseOthers() { var ws = DocumentManager.getWorkingSet(), e = new $.Event("contextmenu"), @@ -144,7 +143,7 @@ define(function (require, exports, module) { runs(function () { expect(DocumentManager.getCurrentDocument()).not.toBe(null); - }); + }); } it("Close others", function () { @@ -152,10 +151,10 @@ define(function (require, exports, module) { cmdToRun = "file.close_others"; runs(runCloseOthers); - - runs(function () { - expect(DocumentManager.getWorkingSet().length).toEqual(1); - }); + + runs(function () { + expect(DocumentManager.getWorkingSet().length).toEqual(1); + }); }); it("Close others above", function () { @@ -163,7 +162,7 @@ define(function (require, exports, module) { cmdToRun = "file.close_above"; runs(runCloseOthers); - + runs(function () { expect(DocumentManager.getWorkingSet().length).toEqual(3); }); From fbdd8772d9ecb166469c1cbdd0b5da66cd6e4322 Mon Sep 17 00:00:00 2001 From: sathyamoorthi Date: Fri, 25 Oct 2013 13:05:45 +0530 Subject: [PATCH 8/8] Test file updated to match standards. --- src/extensions/default/CloseOthers/main.js | 2 +- .../default/CloseOthers/unittests.js | 30 ++++++++++++------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/extensions/default/CloseOthers/main.js b/src/extensions/default/CloseOthers/main.js index ffead36d15e..8353297daa9 100644 --- a/src/extensions/default/CloseOthers/main.js +++ b/src/extensions/default/CloseOthers/main.js @@ -63,7 +63,7 @@ define(function (require, exports, module) { } /* - Pass 'command id' and it'll let you know, whether contextmenu for that command is existing or not. + Pass 'command id' and it'll let you know, whether contextmenu item for that command is existing or not. */ function isMenuThere(cmd) { return Menus.getMenuItem(workingSetCmenu.id + "-" + cmd) ? true : false; diff --git a/src/extensions/default/CloseOthers/unittests.js b/src/extensions/default/CloseOthers/unittests.js index ad5179a626e..92bacedc7fd 100644 --- a/src/extensions/default/CloseOthers/unittests.js +++ b/src/extensions/default/CloseOthers/unittests.js @@ -31,8 +31,7 @@ define(function (require, exports, module) { FileUtils = brackets.getModule("file/FileUtils"), CommandManager, Commands, - Dialogs, - EditorManager, + Menus, DocumentManager; describe("CloseOthers", function () { @@ -86,9 +85,8 @@ define(function (require, exports, module) { brackets = testWindow.brackets; DocumentManager = testWindow.brackets.test.DocumentManager; CommandManager = testWindow.brackets.test.CommandManager; - EditorManager = testWindow.brackets.test.EditorManager; - Dialogs = testWindow.brackets.test.Dialogs; - Commands = testWindow.brackets.test.Commands; + Commands = testWindow.brackets.test.Commands; + Menus = testWindow.brackets.test.Menus; }); }); @@ -119,22 +117,26 @@ define(function (require, exports, module) { testWindow = null; $ = null; brackets = null; - EditorManager = null; SpecRunnerUtils.closeTestWindow(); }); function runCloseOthers() { var ws = DocumentManager.getWorkingSet(), - e = new $.Event("contextmenu"), + cm = Menus.getContextMenu(Menus.ContextMenuIds.WORKING_SET_MENU), promise; if (ws.length > docSelectIndex) { DocumentManager.getDocumentForPath(ws[docSelectIndex].fullPath).done(function (doc) { DocumentManager.setCurrentDocument(doc); - - e.pageX = 20; - e.pageY = 20; - $("#open-files-container").trigger(e); + + /* + "Close Others" extension removes unnecessary menus AND add necessary menus on workingset by listening + `beforeContextMenuOpen` event. For that, we have to open workinget's context menu. Then only this extension + will populate necessary context menu items. + + `pageX` and `pageY` can be any number. our only intention is to open context menu. but it can open at anywhere. + */ + cm.open({pageX: 0, pageY: 0}); }); promise = CommandManager.execute(cmdToRun); @@ -152,6 +154,8 @@ define(function (require, exports, module) { runs(runCloseOthers); + //we created 5 files and selected 3rd file (index = 2), then we ran "close others". + //At this point we should have only one file in working set. runs(function () { expect(DocumentManager.getWorkingSet().length).toEqual(1); }); @@ -163,6 +167,8 @@ define(function (require, exports, module) { runs(runCloseOthers); + //we created 5 files and selected 3rd file (index = 2), then we ran "close others above". + //At this point we should have only 3 files in working set. runs(function () { expect(DocumentManager.getWorkingSet().length).toEqual(3); }); @@ -174,6 +180,8 @@ define(function (require, exports, module) { runs(runCloseOthers); + //we created 5 files and selected 2nd file (index = 1), then we ran "close others below". + //At this point we should have only 2 files in working set. runs(function () { expect(DocumentManager.getWorkingSet().length).toEqual(2); });