From dcabd39c314003f6398e4d29b6cb9d32fdc328dd Mon Sep 17 00:00:00 2001 From: Bernhard Sirlinger Date: Fri, 21 Jun 2013 16:25:08 +0200 Subject: [PATCH 1/4] Made name for Command optional --- src/command/CommandManager.js | 6 +++--- src/document/DocumentCommandHandlers.js | 6 +++--- src/nls/root/strings.js | 6 ------ 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/command/CommandManager.js b/src/command/CommandManager.js index 8592c25916b..af29d96f1ea 100644 --- a/src/command/CommandManager.js +++ b/src/command/CommandManager.js @@ -163,7 +163,7 @@ define(function (require, exports, module) { /** * Registers a global command. - * @param {string} name - text that will be displayed in the UI to represent command + * @param {?string} name - optional text that will be displayed in the UI to represent command * @param {string} id - unique identifier for command. * Core commands in Brackets use a simple command title as an id, for example "open.file". * Extensions should use the following format: "author.myextension.mycommandname". @@ -179,8 +179,8 @@ define(function (require, exports, module) { console.log("Attempting to register an already-registered command: " + id); return null; } - if (!name || !id || !commandFn) { - console.error("Attempting to register a command with a missing name, id, or command function:" + name + " " + id); + if (!id || !commandFn) { + console.error("Attempting to register a command with a missing id, or command function:" + name + " " + id); return null; } diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index c28b64f73f3..a864bd5ce03 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -1110,7 +1110,7 @@ define(function (require, exports, module) { CommandManager.register(Strings.CMD_FILE_CLOSE, Commands.FILE_CLOSE, handleFileClose); CommandManager.register(Strings.CMD_FILE_CLOSE_ALL, Commands.FILE_CLOSE_ALL, handleFileCloseAll); - CommandManager.register(Strings.CMD_CLOSE_WINDOW, Commands.FILE_CLOSE_WINDOW, handleFileCloseWindow); + CommandManager.register(null, Commands.FILE_CLOSE_WINDOW, handleFileCloseWindow); if (brackets.platform === "win") { CommandManager.register(Strings.CMD_EXIT, Commands.FILE_QUIT, handleFileQuit); @@ -1118,8 +1118,8 @@ define(function (require, exports, module) { CommandManager.register(Strings.CMD_QUIT, Commands.FILE_QUIT, handleFileQuit); } - CommandManager.register(Strings.CMD_ABORT_QUIT, Commands.APP_ABORT_QUIT, _handleAbortQuit); - CommandManager.register(Strings.CMD_BEFORE_MENUPOPUP, Commands.APP_BEFORE_MENUPOPUP, _handleBeforeMenuPopup); + CommandManager.register(null, Commands.APP_ABORT_QUIT, _handleAbortQuit); + CommandManager.register(null, Commands.APP_BEFORE_MENUPOPUP, _handleBeforeMenuPopup); CommandManager.register(Strings.CMD_NEXT_DOC, Commands.NAVIGATE_NEXT_DOC, handleGoNextDoc); CommandManager.register(Strings.CMD_PREV_DOC, Commands.NAVIGATE_PREV_DOC, handleGoPrevDoc); diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index e8bbb07244e..d095a8b8cff 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -260,12 +260,6 @@ define({ "CMD_TWITTER" : "{TWITTER_NAME} on Twitter", "CMD_ABOUT" : "About {APP_TITLE}", - - // Special commands invoked by the native shell - "CMD_CLOSE_WINDOW" : "Close Window", - "CMD_ABORT_QUIT" : "Abort Quit", - "CMD_BEFORE_MENUPOPUP" : "Before Menu Popup", - // Strings for main-view.html "EXPERIMENTAL_BUILD" : "experimental build", "DEVELOPMENT_BUILD" : "development build", From 5abf011ade0543092dde2a208a1fc94f0363c88e Mon Sep 17 00:00:00 2001 From: Bernhard Sirlinger Date: Wed, 31 Jul 2013 20:24:20 +0200 Subject: [PATCH 2/4] Whitespace cleanup --- src/document/DocumentCommandHandlers.js | 46 ++++++++++++------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index 0e287a3507e..3da0808478e 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -1,24 +1,24 @@ /* * Copyright (c) 2012 Adobe Systems Incorporated. All rights reserved. - * + * * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the * Software is furnished to do so, subject to the following conditions: - * + * * The above copyright notice and this permission notice shall be included in * all copies or substantial portions of the Software. - * + * * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER * DEALINGS IN THE SOFTWARE. - * + * */ @@ -119,7 +119,7 @@ define(function (require, exports, module) { /** * Returns a short title for a given document. * - * @param {Document} doc + * @param {Document} doc * @return {string} - a short title for doc. */ function _shortTitleForDocument(doc) { @@ -216,11 +216,11 @@ define(function (require, exports, module) { /** * @private - * Creates a document and displays an editor for the specified file path. + * Creates a document and displays an editor for the specified file path. * If no path is specified, a file prompt is provided for input. * @param {?string} fullPath - The path of the file to open; if it's null we'll prompt for it * @param {boolean=} silent - If true, don't show error message - * @return {$.Promise} a jQuery promise that will be resolved with a new + * @return {$.Promise} a jQuery promise that will be resolved with a new * document for the specified file path, or rejected if the file can not be read. */ function _doOpenWithOptionalPath(fullPath, silent) { @@ -269,12 +269,12 @@ define(function (require, exports, module) { * @private * Splits a decorated file path into its parts. * @param {?string} path - a string of the form "fullpath[:lineNumber[:columnNumber]]" - * @return {{path: string, line: ?number, column: ?number}} + * @return {{path: string, line: ?number, column: ?number}} */ function _parseDecoratedPath(path) { var result = {path: path, line: null, column: null}; if (path) { - // If the path has a trailing :lineNumber and :columnNumber, strip + // If the path has a trailing :lineNumber and :columnNumber, strip // these off and assign to result.line and result.column. var matchResult = /(.+?):([0-9]+)(:([0-9]+))?$/.exec(path); if (matchResult) { @@ -322,7 +322,7 @@ define(function (require, exports, module) { // from command line: ...../Brackets.app/Contents path - where 'path' is undecorated // from command line: ...../Brackets.app path - where 'path' has the form "path:line" // from command line: ...../Brackets.app path - where 'path' has the form "path:line:column" - // from command line: open -a ...../Brackets.app path - where 'path' is undecorated + // from command line: open -a ...../Brackets.app path - where 'path' is undecorated // do "View Source" from Adobe Scout version 1.2 or newer (this will use decorated paths of the form "path:line:column") } @@ -345,7 +345,7 @@ define(function (require, exports, module) { * @param {string} baseFileName The base to start with, "-n" will get appened to make unique * @param {string} fileExt The file extension * @param {boolean} isFolder True if the suggestion is for a folder name - * @return {$.Promise} a jQuery promise that will be resolved with a unique name starting with + * @return {$.Promise} a jQuery promise that will be resolved with a unique name starting with * the given base name */ function _getUntitledFileSuggestion(dir, baseFileName, fileExt, isFolder) { @@ -1200,7 +1200,7 @@ define(function (require, exports, module) { CommandManager.register(Strings.CMD_FILE_CLOSE, Commands.FILE_CLOSE, handleFileClose); CommandManager.register(Strings.CMD_FILE_CLOSE_ALL, Commands.FILE_CLOSE_ALL, handleFileCloseAll); - CommandManager.register(null, Commands.FILE_CLOSE_WINDOW, handleFileCloseWindow); + CommandManager.register(null, Commands.FILE_CLOSE_WINDOW, handleFileCloseWindow); if (brackets.platform === "win") { CommandManager.register(Strings.CMD_EXIT, Commands.FILE_QUIT, handleFileQuit); @@ -1208,8 +1208,8 @@ define(function (require, exports, module) { CommandManager.register(Strings.CMD_QUIT, Commands.FILE_QUIT, handleFileQuit); } - CommandManager.register(null, Commands.APP_ABORT_QUIT, _handleAbortQuit); - CommandManager.register(null, Commands.APP_BEFORE_MENUPOPUP, _handleBeforeMenuPopup); + CommandManager.register(null, Commands.APP_ABORT_QUIT, _handleAbortQuit); + CommandManager.register(null, Commands.APP_BEFORE_MENUPOPUP, _handleBeforeMenuPopup); CommandManager.register(Strings.CMD_NEXT_DOC, Commands.NAVIGATE_NEXT_DOC, handleGoNextDoc); CommandManager.register(Strings.CMD_PREV_DOC, Commands.NAVIGATE_PREV_DOC, handleGoPrevDoc); @@ -1220,6 +1220,6 @@ define(function (require, exports, module) { $(DocumentManager).on("dirtyFlagChange", handleDirtyChange); $(DocumentManager).on("currentDocumentChange fileNameChange", updateDocumentTitle); - // Reset the untitled document counter before changing projects + // Reset the untitled document counter before changing projects $(ProjectManager).on("beforeProjectClose", function () { _nextUntitledIndexToUse = 1; }); }); From 1c1a1379553da1668f5b3ca18eed7a341e9d97e9 Mon Sep 17 00:00:00 2001 From: Bernhard Sirlinger Date: Sat, 7 Sep 2013 17:46:24 +0200 Subject: [PATCH 3/4] Refactor to use special registerInternal API --- src/command/CommandManager.js | 83 +++++++++++++++++-------- src/document/DocumentCommandHandlers.js | 9 +-- 2 files changed, 62 insertions(+), 30 deletions(-) diff --git a/src/command/CommandManager.js b/src/command/CommandManager.js index af29d96f1ea..3e59362f9b2 100644 --- a/src/command/CommandManager.js +++ b/src/command/CommandManager.js @@ -1,24 +1,24 @@ /* * Copyright (c) 2012 Adobe Systems Incorporated. All rights reserved. - * + * * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the * Software is furnished to do so, subject to the following conditions: - * + * * The above copyright notice and this permission notice shall be included in * all copies or substantial portions of the Software. - * + * * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER * DEALINGS IN THE SOFTWARE. - * + * */ @@ -102,7 +102,7 @@ define(function (require, exports, module) { return this._enabled; }; - /** + /** * Sets enabled state of Command and dispatches "enabledStateChange" * when the enabled state changes. * @param {boolean} enabled @@ -116,7 +116,7 @@ define(function (require, exports, module) { } }; - /** + /** * Sets enabled state of Command and dispatches "checkedStateChange" * when the enabled state changes. * @param {boolean} checked @@ -138,11 +138,11 @@ define(function (require, exports, module) { /** * Sets the name of the Command and dispatches "nameChange" so that * UI that reflects the command name can update. - * + * * Note, a Command name can appear in either HTML or native UI * so HTML tags should not be used. To add a Unicode character, * use \uXXXX instead of an HTML entity. - * + * * @param {string} name */ Command.prototype.setName = function (name) { @@ -163,10 +163,10 @@ define(function (require, exports, module) { /** * Registers a global command. - * @param {?string} name - optional text that will be displayed in the UI to represent command + * @param {string} name - text that will be displayed in the UI to represent command * @param {string} id - unique identifier for command. * Core commands in Brackets use a simple command title as an id, for example "open.file". - * Extensions should use the following format: "author.myextension.mycommandname". + * Extensions should use the following format: "author.myextension.mycommandname". * For example, "lschmitt.csswizard.format.css". * @param {function(...)} commandFn - the function to call when the command is executed. Any arguments passed to * execute() (after the id) are passed as arguments to the function. If the function is asynchronous, @@ -179,8 +179,8 @@ define(function (require, exports, module) { console.log("Attempting to register an already-registered command: " + id); return null; } - if (!id || !commandFn) { - console.error("Attempting to register a command with a missing id, or command function:" + name + " " + id); + if (!name || !id || !commandFn) { + console.error("Attempting to register a command with a missing name, id, or command function:" + name + " " + id); return null; } @@ -192,6 +192,36 @@ define(function (require, exports, module) { return command; } + /** + * Registers a global internal only command. + * @param {string} id - unique identifier for command. + * Core commands in Brackets use a simple command title as an id, for example "open.file". + * Extensions should use the following format: "author.myextension.mycommandname". + * For example, "lschmitt.csswizard.format.css". + * @param {function(...)} commandFn - the function to call when the command is executed. Any arguments passed to + * execute() (after the id) are passed as arguments to the function. If the function is asynchronous, + * it must return a jQuery promise that is resolved when the command completes. Otherwise, the + * CommandManager will assume it is synchronous, and return a promise that is already resolved. + * @return {?Command} + */ + function registerInternal(id, commandFn) { + if (_commands[id]) { + console.log("Attempting to register an already-registered command: " + id); + return null; + } + if (!id || !commandFn) { + console.error("Attempting to register an internal command with a missing id, or command function: " + id); + return null; + } + + var command = new Command(null, id, commandFn); + _commands[id] = command; + + $(exports).triggerHandler("commandRegistered", [command]); + + return command; + } + /** * Clear all commands for unit testing, but first make copy of commands so that * they can be restored afterward @@ -249,10 +279,11 @@ define(function (require, exports, module) { } // Define public API - exports.register = register; - exports.execute = execute; - exports.get = get; - exports.getAll = getAll; - exports._testReset = _testReset; - exports._testRestore = _testRestore; + exports.register = register; + exports.registerInternal = registerInternal; + exports.execute = execute; + exports.get = get; + exports.getAll = getAll; + exports._testReset = _testReset; + exports._testRestore = _testRestore; }); diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index 3da0808478e..ff57a6b868c 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -1200,7 +1200,6 @@ define(function (require, exports, module) { CommandManager.register(Strings.CMD_FILE_CLOSE, Commands.FILE_CLOSE, handleFileClose); CommandManager.register(Strings.CMD_FILE_CLOSE_ALL, Commands.FILE_CLOSE_ALL, handleFileCloseAll); - CommandManager.register(null, Commands.FILE_CLOSE_WINDOW, handleFileCloseWindow); if (brackets.platform === "win") { CommandManager.register(Strings.CMD_EXIT, Commands.FILE_QUIT, handleFileQuit); @@ -1208,14 +1207,16 @@ define(function (require, exports, module) { CommandManager.register(Strings.CMD_QUIT, Commands.FILE_QUIT, handleFileQuit); } - CommandManager.register(null, Commands.APP_ABORT_QUIT, _handleAbortQuit); - CommandManager.register(null, Commands.APP_BEFORE_MENUPOPUP, _handleBeforeMenuPopup); - CommandManager.register(Strings.CMD_NEXT_DOC, Commands.NAVIGATE_NEXT_DOC, handleGoNextDoc); CommandManager.register(Strings.CMD_PREV_DOC, Commands.NAVIGATE_PREV_DOC, handleGoPrevDoc); CommandManager.register(Strings.CMD_SHOW_IN_TREE, Commands.NAVIGATE_SHOW_IN_FILE_TREE, handleShowInTree); CommandManager.register(Strings.CMD_SHOW_IN_OS, Commands.NAVIGATE_SHOW_IN_OS, handleShowInOS); + // Those commands have no UI representation, and are only used internally + CommandManager.registerInternal(Commands.APP_ABORT_QUIT, _handleAbortQuit); + CommandManager.registerInternal(Commands.APP_BEFORE_MENUPOPUP, _handleBeforeMenuPopup); + CommandManager.registerInternal(Commands.FILE_CLOSE_WINDOW, handleFileCloseWindow); + // Listen for changes that require updating the editor titlebar $(DocumentManager).on("dirtyFlagChange", handleDirtyChange); $(DocumentManager).on("currentDocumentChange fileNameChange", updateDocumentTitle); From 8ed897788ba1f0a2f513620f284ba7b2a2964c15 Mon Sep 17 00:00:00 2001 From: Bernhard Sirlinger Date: Sun, 8 Sep 2013 18:37:17 +0200 Subject: [PATCH 4/4] Code Review fix --- src/command/CommandManager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/command/CommandManager.js b/src/command/CommandManager.js index 3e59362f9b2..0775cf55a76 100644 --- a/src/command/CommandManager.js +++ b/src/command/CommandManager.js @@ -195,7 +195,7 @@ define(function (require, exports, module) { /** * Registers a global internal only command. * @param {string} id - unique identifier for command. - * Core commands in Brackets use a simple command title as an id, for example "open.file". + * Core commands in Brackets use a simple command title as an id, for example "app.abort_quit". * Extensions should use the following format: "author.myextension.mycommandname". * For example, "lschmitt.csswizard.format.css". * @param {function(...)} commandFn - the function to call when the command is executed. Any arguments passed to