From a0bb23020cde1c12264e2aa4eee314c010310d26 Mon Sep 17 00:00:00 2001 From: Simon Lydell Date: Sun, 11 Oct 2015 18:41:08 +0200 Subject: [PATCH] Improve keyboard shortcut overriding behavior - When the feature was initially implemented, it was modeled after putting a series of `map` commands after each other in .vimrc, which is written in a procedural programming language. Then it makes sense that latter `map` calls override earlier ones. That's why VimFx used to let commands further down the list in VimFx's settings page in the Add-ons Manager override earlier commands. The more I used it, though, that didn't feel entirely natural. After a while I realized that in such an interface it makes sense having _earlier_ commands win. With this commit, latter commands _don't_ override earlier ones anymore, and are ignored, showing an error message. - The above point also means that editing a shortcut can never result in error messages appearing _above_ that command. If you add `g` to a shortcut near the end of the list, there used to appear an error message for every shortcut starting with `g` above it (which is quite a few), causing dis-orientation when the focused text box was moved down because of all those error messages. - With this commit, the order of the modes, categories and commands is taken into account. That used to work only when the user hadn't customized the orders or hadn't added new modes, categories and/or commands. - A small fix for `` is also included. If a shortcut contains invalid usage of `` it is now properly ignored (still showing an error message). --- documentation/api.md | 28 +++++++++++++++++++---- extension/lib/vimfx.coffee | 41 +++++++++++++++++++++------------- extension/test/test-api.coffee | 2 +- 3 files changed, 51 insertions(+), 20 deletions(-) diff --git a/documentation/api.md b/documentation/api.md index 7926942..2fd7257 100644 --- a/documentation/api.md +++ b/documentation/api.md @@ -68,7 +68,26 @@ Sets the value of the VimFx pref `pref` to `value`. // Set the value of the Hint chars option: vimfx.set('hint_chars', 'abcdefghijklmnopqrstuvwxyz') // Add yet a keyboard shortcut for the `f` command: -vimfx.set('modes.normal.follow', vimfx.get('modes.normal.follow') + ' e'); +vimfx.set('modes.normal.follow', vimfx.get('modes.normal.follow') + ' e') +``` + +Note: If you produce conflicting keyboard shortcuts, the order of your code does +not matter. The command that comes first in VimFx’s settings page in the Add-ons +Manager (and in the help dialog) gets the shortcut; the other one(s) do(es) not. +See the notes about order in [mode object], [category object] and [command +object] for more information about order. + +```js +// Even though we set the shortcut for focusing the search bar last, the command +// for focusing the location bar “wins”, because it comes first in VimFx’s +// settings page in the Add-ons Manager. +vimfx.set('modes.normal.focus_location_bar', 'ö') +vimfx.set('modes.normal.focus_search_bar', 'ö') + +// Swapping their orders also swaps the “winner”. +let {commands} = vimfx.modes.normal +;[commands.focus_location_bar.order, commands.focus_search_bar.order] = + [commands.focus_search_bar.order, commands.focus_location_bar.order] ``` ### `vimfx.addCommand(options, fn)` @@ -280,8 +299,8 @@ categories.custom = { } // Swap the order of the Location and Tabs categories. -;[categories.location.order, categories.tabs.order] = - [categories.tabs.order, categories.location.order] +;[commands.focus_location_bar.order, categories.tabs.order] = + [categories.tabs.order, commands.focus_location_bar.order] ``` ### Mode object @@ -393,7 +412,8 @@ A `match` object has the following properties: the currently focused element does not appear to respond to keystrokes in any special way. -- command: `null` unless `type` is `'full'`. Then it is the matched command. +- command: `null` unless `type` is `'full'`. Then it is the matched command (a + [command object]). The matched command should usually be run at this point. It is suitable to pass on the object passed to [onInput] to the command. Some modes might choose diff --git a/extension/lib/vimfx.coffee b/extension/lib/vimfx.coffee index 12e6fe2..15a176b 100644 --- a/extension/lib/vimfx.coffee +++ b/extension/lib/vimfx.coffee @@ -49,7 +49,7 @@ class VimFx extends utils.EventEmitter @count = '' createKeyTrees: -> - {@keyTrees, @forceCommands, @errors} = createKeyTrees(@modes) + {@keyTrees, @forceCommands, @errors} = createKeyTrees(@getGroupedCommands()) stringifyKeyEvent: (event) -> return notation.stringify(event, { @@ -156,7 +156,7 @@ byOrder = (a, b) -> a.order - b.order class Leaf constructor: (@command, @originalSequence) -> -createKeyTrees = (modes) -> +createKeyTrees = (groupedCommands) -> keyTrees = {} errors = {} forceCommands = {} @@ -165,13 +165,12 @@ createKeyTrees = (modes) -> (errors[command.pref] ?= []).push(error) pushOverrideErrors = (command, tree) -> - for { command: overriddenCommand, originalSequence } in getLeaves(tree) - error = - id: 'overridden_by' - subject: command.description() - context: originalSequence - pushError(error, overriddenCommand) - return + { command: overridingCommand, originalSequence } = getFirstLeaf(tree) + error = + id: 'overridden_by' + subject: overridingCommand.description() + context: originalSequence + pushError(error, command) pushForceKeyError = (command, originalSequence) -> error = @@ -180,41 +179,47 @@ createKeyTrees = (modes) -> context: originalSequence pushError(error, command) - for modeName, { commands } of modes - keyTrees[modeName] = {} - for commandName, command of commands + for mode in groupedCommands + keyTrees[mode._name] = {} + for category in mode.categories then for { command } in category.commands { shortcuts, errors: parseErrors } = parseShortcutPref(command.pref) pushError(error, command) for error in parseErrors command._sequences = [] for shortcut in shortcuts [ prefixKeys..., lastKey ] = shortcut.normalized - tree = keyTrees[modeName] + tree = keyTrees[mode._name] command._sequences.push(shortcut.original) + errored = false for prefixKey, index in prefixKeys if prefixKey == FORCE_KEY if index == 0 forceCommands[command.pref] = true + continue else pushForceKeyError(command, shortcut.original) - continue + errored = true + break if prefixKey of tree next = tree[prefixKey] if next instanceof Leaf pushOverrideErrors(command, next) - tree = tree[prefixKey] = {} + errored = true + break else tree = next else tree = tree[prefixKey] = {} + continue if errored if lastKey == FORCE_KEY pushForceKeyError(command, shortcut.original) continue if lastKey of tree pushOverrideErrors(command, tree[lastKey]) + continue tree[lastKey] = new Leaf(command, shortcut.original) return {keyTrees, forceCommands, errors} @@ -243,6 +248,12 @@ parseShortcutPref = (pref) -> return {shortcuts, errors} +getFirstLeaf = (node) -> + if node instanceof Leaf + return node + for key, value of node + return getFirstLeaf(value) + getLeaves = (node) -> if node instanceof Leaf return [node] diff --git a/extension/test/test-api.coffee b/extension/test/test-api.coffee index 0df2302..bed4d5d 100644 --- a/extension/test/test-api.coffee +++ b/extension/test/test-api.coffee @@ -106,7 +106,7 @@ exports['test customization'] = (assert, passed_vimfx) -> getAPI((vimfx) -> assert.equal(category_new.name, 'New category') [ test_command ] = category_new.commands assert.equal(test_command.command.description(), 'Test ignore mode command') - assert.deepEqual(test_command.enabledSequences, ['<ö>']) + assert.deepEqual(test_command.enabledSequences, ['ö']) # Remove the added commands. delete vimfx.modes.normal.commands.test_command -- 2.39.3