From 42c9af11499bfe6e68f56fb60017aa19b000ceaf Mon Sep 17 00:00:00 2001 From: abose Date: Thu, 28 Dec 2023 21:44:54 +0530 Subject: [PATCH] fix: key shortcut display issues and file recovery errors --- src/base-config/keyboard.json | 4 +-- src/command/KeyBindingManager.js | 21 ++++++++++++--- .../NavigationAndHistory/FileRecovery.js | 12 ++++++--- .../NavigationProvider.js | 27 ++++++++++++------- .../NavigationAndHistory/keyboard.json | 6 +++-- src/extensions/default/Phoenix/new-project.js | 2 +- 6 files changed, 50 insertions(+), 22 deletions(-) diff --git a/src/base-config/keyboard.json b/src/base-config/keyboard.json index 828ce856c..3eceec1a0 100644 --- 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 index b8d99fffe..32abe62d0 100644 --- 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-' and 'Alt-Shift-' 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 index f34e38c4a..c6999c567 100644 --- 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 index a99d4ceef..7e51ef0ca 100644 --- 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 index 4b0dfc81e..8a72e0042 100644 --- 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 index 187d18091..13204fea2 100644 --- 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() {