fix: key shortcut display issues and file recovery errors

phcode-dev · Dec 28, 2023 · 42c9af1 · 42c9af1
1 parent 106d430
commit 42c9af1
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 22 deletions.
diff --git a/src/base-config/keyboard.json b/src/base-config/keyboard.json
@@ -44,7 +44,7 @@
         "Ctrl-Q"
     ],
     "file.duplicateFile": [
-        "Alt-Shift-D"
+        "Ctrl-2"
     ],
     "edit.undo": [
         "Ctrl-Z"
@@ -287,7 +287,7 @@
         "Ctrl-Shift-O"
     ],
     "navigate.showInFileTree":  [
-        "Ctrl-Shift-;"
+        "Ctrl-Shift-0"
     ],
     "navigate.gotoLine":  [
         {

diff --git a/src/command/KeyBindingManager.js b/src/command/KeyBindingManager.js
@@ -51,6 +51,9 @@ define(function (require, exports, module) {
     let KEYMAP_FILENAME     = "keymap.json",
         _userKeyMapFilePath = path.normalize(brackets.app.getApplicationSupportDirectory() + "/" + KEYMAP_FILENAME);
 
+    const EVENT_KEY_BINDING_ADDED = "keyBindingAdded",
+        EVENT_KEY_BINDING_REMOVED = "keyBindingRemoved";
+
     /**
      * @private
      * Maps normalized shortcut descriptor to key binding info.
@@ -569,7 +572,7 @@ define(function (require, exports, module) {
                 });
 
                 if (command) {
-                    command.trigger("keyBindingRemoved", {key: normalizedKey, displayKey: binding.displayKey});
+                    command.trigger(EVENT_KEY_BINDING_REMOVED, {key: normalizedKey, displayKey: binding.displayKey});
                 }
             }
         }
@@ -649,9 +652,17 @@ define(function (require, exports, module) {
 
         // skip if the key binding is invalid
         if (!normalized) {
-            console.error("Unable to parse key binding " + key + ". Permitted modifiers: Ctrl, Cmd, Alt, Opt, Shift; separated by '-' (not '+').");
+            console.error(`Unable to parse key binding '${key}' for command '${commandID}'. Permitted modifiers: Ctrl, Cmd, Alt, Opt, Shift; separated by '-' (not '+').`);
             return null;
         }
+        function isSingleCharAZ(str) {
+            return /^[A-Z]$/i.test(str);
+        }
+        const keySplit = normalized.split("-");
+        if((keySplit.length ===2 && keySplit[0] === 'Alt' && isSingleCharAZ(keySplit[1])) ||
+            (keySplit.length ===3 && keySplit[0] === 'Alt' && keySplit[1] === 'Shift' && isSingleCharAZ(keySplit[2]))){
+            console.error(`Key binding '${normalized}' for command '${commandID}' may cause issues. The key combinations starting with 'Alt-<letter>' and 'Alt-Shift-<letter>' are reserved. On macOS, they are used for AltGr internationalization, and on Windows/Linux, they are used for menu navigation shortcuts.`);
+        }
 
         // check for duplicate key bindings
         existing = _keyMap[normalized];
@@ -763,7 +774,7 @@ define(function (require, exports, module) {
         command = CommandManager.get(commandID);
 
         if (command) {
-            command.trigger("keyBindingAdded", result);
+            command.trigger(EVENT_KEY_BINDING_ADDED, result, commandID);
         }
 
         return result;
@@ -1500,6 +1511,10 @@ define(function (require, exports, module) {
     exports.addGlobalKeydownHook = addGlobalKeydownHook;
     exports.removeGlobalKeydownHook = removeGlobalKeydownHook;
 
+    // public events
+    exports.EVENT_KEY_BINDING_ADDED = EVENT_KEY_BINDING_ADDED;
+    exports.EVENT_KEY_BINDING_REMOVED = EVENT_KEY_BINDING_REMOVED;
+
     /**
      * Use windows-specific bindings if no other are found (e.g. Linux).
      * Core Brackets modules that use key bindings should always define at

diff --git a/src/extensions/default/NavigationAndHistory/FileRecovery.js b/src/extensions/default/NavigationAndHistory/FileRecovery.js
@@ -125,7 +125,7 @@ define(function (require, exports, module) {
 
     function getRestoreFilePath(projectFilePath, projectRootPath) {
         if(!projectFilePath.startsWith(projectRootPath) || !trackedProjects[projectRootPath]){
-            console.error(`[recovery] cannot backed up as ${projectRootPath} is not in project ${projectRootPath}`);
+            console.error(`[recovery] cannot backed up as ${projectFilePath} is not in project ${projectRootPath}`);
             return null;
         }
         let pathWithinProject = projectFilePath.replace(projectRootPath, "");
@@ -268,8 +268,10 @@ define(function (require, exports, module) {
         let trackedFilePaths =  Object.keys(project.trackedFileContents);
         for(let trackedFilePath of trackedFilePaths){
             const restorePath = getRestoreFilePath(trackedFilePath, projectRoot.fullPath);
-            const content = project.trackedFileContents[trackedFilePath];
-            await writeFileIgnoreFailure(restorePath, content);
+            if(restorePath) {
+                const content = project.trackedFileContents[trackedFilePath];
+                await writeFileIgnoreFailure(restorePath, content);
+            }
             delete project.trackedFileContents[trackedFilePath];
         }
     }
@@ -280,7 +282,9 @@ define(function (require, exports, module) {
         for(let trackedPath of allTrackingPaths){
             if(!docPathsToTrack[trackedPath]){
                 const restoreFile = getRestoreFilePath(trackedPath, projectRoot.fullPath);
-                await silentlyRemoveFile(restoreFile);
+                if(restoreFile) {
+                    await silentlyRemoveFile(restoreFile);
+                }
                 delete project.trackedFileUpdateTimestamps[trackedPath];
             }
         }

diff --git a/src/extensions/default/NavigationAndHistory/NavigationProvider.js b/src/extensions/default/NavigationAndHistory/NavigationProvider.js
@@ -51,7 +51,8 @@ define(function (require, exports, module) {
     let $navback = null,
         $navForward = null,
         $searchNav = null,
-        $newProject = null;
+        $newProject = null,
+        $showInTree = null;
 
    /**
     * Contains list of most recently known cursor positions.
@@ -660,23 +661,29 @@ define(function (require, exports, module) {
         return baseStr;
     }
 
+    function updateTooltips() {
+        $navback.attr("title", _getShortcutDisplay(Strings.CMD_NAVIGATE_BACKWARD, NAVIGATION_JUMP_BACK));
+        $navForward.attr("title", _getShortcutDisplay(Strings.CMD_NAVIGATE_FORWARD, NAVIGATION_JUMP_FWD));
+        $showInTree.attr("title", _getShortcutDisplay(Strings.CMD_SHOW_IN_TREE, Commands.NAVIGATE_SHOW_IN_FILE_TREE));
+        $searchNav.attr("title", _getShortcutDisplay(Strings.CMD_FIND_IN_FILES, Commands.CMD_FIND_IN_FILES));
+        // new project extension is not yet loaded, so we cant show keyboard shortcut here.
+        $newProject.attr("title", Strings.CMD_PROJECT_NEW);
+    }
+
     function _setupNavigationButtons() {
         let $mainNavBarRight = $("#mainNavBarRight");
         let $mainNavBarLeft = $("#mainNavBarLeft");
-        let $showInTree = $mainNavBarRight.find("#showInfileTree");
+        $showInTree = $mainNavBarRight.find("#showInfileTree");
         $navback = $mainNavBarRight.find("#navBackButton");
         $navForward = $mainNavBarRight.find("#navForwardButton");
         $searchNav = $mainNavBarRight.find("#searchNav");
 
         $newProject = $mainNavBarLeft.find("#newProject");
-
-
-        $navback.attr("title", _getShortcutDisplay(Strings.CMD_NAVIGATE_BACKWARD, NAVIGATION_JUMP_BACK));
-        $navForward.attr("title", _getShortcutDisplay(Strings.CMD_NAVIGATE_FORWARD, NAVIGATION_JUMP_FWD));
-        $showInTree.attr("title", _getShortcutDisplay(Strings.CMD_SHOW_IN_TREE, Commands.NAVIGATE_SHOW_IN_FILE_TREE));
-        $searchNav.attr("title", _getShortcutDisplay(Strings.CMD_FIND_IN_FILES, Commands.CMD_FIND_IN_FILES));
-        // new project extension is not yet loaded, so we cant show keyboard shortcut here.
-        $newProject.attr("title", Strings.CMD_PROJECT_NEW);
+        updateTooltips();
+        CommandManager.get(NAVIGATION_JUMP_BACK).on(KeyBindingManager.EVENT_KEY_BINDING_ADDED, updateTooltips);
+        CommandManager.get(NAVIGATION_JUMP_FWD).on(KeyBindingManager.EVENT_KEY_BINDING_ADDED, updateTooltips);
+        CommandManager.get(Commands.NAVIGATE_SHOW_IN_FILE_TREE).on(KeyBindingManager.EVENT_KEY_BINDING_ADDED, updateTooltips);
+        CommandManager.get(Commands.CMD_FIND_IN_FILES).on(KeyBindingManager.EVENT_KEY_BINDING_ADDED, updateTooltips);
 
         $navback.on("click", _navigateBackClicked);
         $navForward.on("click", _navigateForwardClicked);

diff --git a/src/extensions/default/NavigationAndHistory/keyboard.json b/src/extensions/default/NavigationAndHistory/keyboard.json
@@ -30,12 +30,14 @@
     ],
     "navigation.jump.back":  [
         {
-            "key": "Ctrl-Shift-,"
+            "key": "Alt-Left",
+            "displayKey": "Alt-←"
         }
     ],
     "navigation.jump.fwd":  [
         {
-            "key": "Ctrl-Shift-."
+            "key": "Alt-Right",
+            "displayKey": "Alt-→"
         }
     ]
 }
diff --git a/src/extensions/default/Phoenix/new-project.js b/src/extensions/default/Phoenix/new-project.js
@@ -72,7 +72,7 @@ define(function (require, exports, module) {
     function _addMenuEntries() {
         CommandManager.register(Strings.CMD_PROJECT_NEW, Commands.FILE_NEW_PROJECT, _showNewProjectDialogue);
         const fileMenu = Menus.getMenu(Menus.AppMenuBar.FILE_MENU);
-        fileMenu.addMenuItem(Commands.FILE_NEW_PROJECT, "Alt-Shift-P", Menus.AFTER, Commands.FILE_NEW_FOLDER);
+        fileMenu.addMenuItem(Commands.FILE_NEW_PROJECT, "Ctrl-Shift-2", Menus.AFTER, Commands.FILE_NEW_FOLDER);
     }
 
     function closeDialogue() {