Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Close Others extension improvements. #5497

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 13 additions & 8 deletions src/document/DocumentCommandHandlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -747,7 +748,9 @@ define(function (require, exports, module) {
}
},
false
);
).then(function () {
return savedFiles;
});
}

/**
Expand Down Expand Up @@ -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();
Expand All @@ -1002,7 +1007,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);
}
});

Expand All @@ -1018,11 +1023,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);
}

/**
Expand Down
98 changes: 71 additions & 27 deletions src/extensions/default/CloseOthers/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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 item 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),
closeOthersAbove = (targetIndex > 0),
closeOthersBelow = (targetIndex < dm.getWorkingSet().length - 1);

if (closeOthersAbove && 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);
}
}
});
});
5 changes: 0 additions & 5 deletions src/extensions/default/CloseOthers/settings.json

This file was deleted.

40 changes: 28 additions & 12 deletions src/extensions/default/CloseOthers/unittests.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ define(function (require, exports, module) {
FileUtils = brackets.getModule("file/FileUtils"),
CommandManager,
Commands,
Dialogs,
EditorManager,
Menus,
DocumentManager;

describe("CloseOthers", function () {
Expand Down Expand Up @@ -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;
});
});

Expand Down Expand Up @@ -119,42 +117,58 @@ define(function (require, exports, module) {
testWindow = null;
$ = null;
brackets = null;
EditorManager = null;
SpecRunnerUtils.closeTestWindow();
});


function runCloseOthers() {
var ws = DocumentManager.getWorkingSet(),
cm = Menus.getContextMenu(Menus.ContextMenuIds.WORKING_SET_MENU),
promise;

if (ws.length > docSelectIndex) {
DocumentManager.getDocumentForPath(ws[docSelectIndex].fullPath).done(function (doc) {
DocumentManager.setCurrentDocument(doc);

/*
"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});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JeffryBooher @redmunds. I replaced triggering contextmenu event with contextmenu.open. Hope you may like it this time.

});

promise = CommandManager.execute(cmdToRun);
waitsForDone(promise, cmdToRun);
}

runs(function () {
expect(DocumentManager.getCurrentDocument()).not.toBe(null);
});
}

it("Close others", function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you fix the tabs in this block. lines 156-159 are misaligned

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is funny issue. It looked good in Brackets and Submlime but not in github. I deleted and recreated those lines to fix this.

docSelectIndex = 2;
cmdToRun = "file.close_others";

runs(runCloseOthers);

runs(function () {
expect(DocumentManager.getWorkingSet().length).toEqual(1);
});

//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);
});
});

it("Close others above", function () {
docSelectIndex = 2;
cmdToRun = "file.close_above";

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);
});
Expand All @@ -166,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);
});
Expand Down