From 94fd11b4ea22c63e459a1f6534aae2b6e9de7ff0 Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Sun, 16 Oct 2016 18:24:18 -0700 Subject: [PATCH 01/15] catching modifier keyups beyond pending timeout still very WIP, lots of pieces still on the floor --- spec/helpers-spec.coffee | 18 +++++++++++- spec/key-binding-spec.coffee | 54 ++++++++++++++++++++++++++++++++++++ src/helpers.coffee | 12 ++++++++ src/key-binding.coffee | 30 +++++++++++++++++++- src/keymap-manager.coffee | 43 ++++++++++++++++++++++++---- 5 files changed, 149 insertions(+), 8 deletions(-) create mode 100644 spec/key-binding-spec.coffee diff --git a/spec/helpers-spec.coffee b/spec/helpers-spec.coffee index 2129f32..6bdb47d 100644 --- a/spec/helpers-spec.coffee +++ b/spec/helpers-spec.coffee @@ -1,4 +1,4 @@ -{normalizeKeystrokes, keystrokesMatch} = require '../src/helpers' +{normalizeKeystrokes, keystrokesMatch, isModifierKeyup} = require '../src/helpers' describe ".normalizeKeystrokes(keystrokes)", -> it "parses and normalizes the keystrokes", -> @@ -61,3 +61,19 @@ describe ".keystrokesMatch(bindingKeystrokes, userKeystrokes)", -> it "returns 'keydownExact' for bindings that match and contain a remainder of only keyup events", -> assert.equal(keystrokesMatch(['a', 'b', '^b'], ['a', 'b']), 'keydownExact') + +describe ".isModifierKeyup(keystroke)", -> + it "returns true for single modifier keyups", -> + assert(isModifierKeyup('^ctrl')) + assert(isModifierKeyup('^shift')) + assert(isModifierKeyup('^alt')) + assert(isModifierKeyup('^cmd')) + assert(isModifierKeyup('^ctrl-shift')) + assert(isModifierKeyup('^alt-cmd')) + it "returns false for modifier keydowns", -> + assert(!isModifierKeyup('ctrl-x')) + assert(!isModifierKeyup('shift-x')) + assert(!isModifierKeyup('alt-x')) + assert(!isModifierKeyup('cmd-x')) + assert(!isModifierKeyup('ctrl-shift-x')) + assert(!isModifierKeyup('alt-cmd-x')) diff --git a/spec/key-binding-spec.coffee b/spec/key-binding-spec.coffee new file mode 100644 index 0000000..61965a4 --- /dev/null +++ b/spec/key-binding-spec.coffee @@ -0,0 +1,54 @@ +KeyBinding = require '../src/key-binding' + +describe "KeyBinding", -> + + describe "is_matched_modifer_keydown_keyup", -> + + describe "returns false when the binding...", -> + it "has no modifier keys", -> + kb = new KeyBinding('test', 'whatever', 'a', 'body', 0) + assert(not kb.is_matched_modifer_keydown_keyup()) + it "has no keyups", -> + kb = new KeyBinding('test', 'whatever', 'ctrl-a', 'body', 0) + assert(not kb.is_matched_modifer_keydown_keyup()) + it "is a bare modifier", -> + kb = new KeyBinding('test', 'whatever', 'ctrl', 'body', 0) + assert(not kb.is_matched_modifer_keydown_keyup()) + it "is a bare modifier keyup", -> + kb = new KeyBinding('test', 'whatever', '^ctrl', 'body', 0) + assert(not kb.is_matched_modifer_keydown_keyup()) + it "has mismatched last_keystroke: ctrl ^shift", -> + kb = new KeyBinding('test', 'whatever', 'ctrl-a ^shift', 'body', 0) + assert(not kb.is_matched_modifer_keydown_keyup()) + it "has mismatched last_keystroke: cmd ^alt", -> + kb = new KeyBinding('test', 'whatever', 'cmd-a ^alt', 'body', 0) + assert(not kb.is_matched_modifer_keydown_keyup()) + it "has partially mismatched last_keystroke: ctrl-cmd ^ctrl", -> + kb = new KeyBinding('test', 'whatever', 'ctrl-cmd-a ^ctrl', 'body', 0) + assert(not kb.is_matched_modifer_keydown_keyup()) + it "has partially mismatched last_keystroke: ctrl-cmd ^cmd", -> + kb = new KeyBinding('test', 'whatever', 'ctrl-cmd-a ^cmd', 'body', 0) + assert(not kb.is_matched_modifer_keydown_keyup()) + it "has partially mismatched last_keystroke: ctrl ^ctrl-shift", -> + kb = new KeyBinding('test', 'whatever', 'ctrl ^ctrl-shift', 'body', 0) + assert(not kb.is_matched_modifer_keydown_keyup()) + + describe "returns true when the binding...", -> + it "has a matched ctrl", -> + kb = new KeyBinding('test', 'whatever', 'ctrl-a ^ctrl', 'body', 0) + assert(kb.is_matched_modifer_keydown_keyup()) + it "has a matched shift", -> + kb = new KeyBinding('test', 'whatever', 'shift-a ^shift', 'body', 0) + assert(kb.is_matched_modifer_keydown_keyup()) + it "has a matched alt", -> + kb = new KeyBinding('test', 'whatever', 'alt-a ^alt', 'body', 0) + assert(kb.is_matched_modifer_keydown_keyup()) + it "has a matched cmd", -> + kb = new KeyBinding('test', 'whatever', 'cmd-a ^cmd', 'body', 0) + assert(kb.is_matched_modifer_keydown_keyup()) + it "has a matched ctrl-shift", -> + kb = new KeyBinding('test', 'whatever', 'ctrl-shift-a ^ctrl-shift', 'body', 0) + assert(kb.is_matched_modifer_keydown_keyup()) + it "has matched bare last_keystroke", -> + kb = new KeyBinding('test', 'whatever', 'ctrl ^ctrl', 'body', 0) + assert(kb.is_matched_modifer_keydown_keyup()) diff --git a/src/helpers.coffee b/src/helpers.coffee index 0465c8a..f79910e 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -187,6 +187,8 @@ nonAltModifiedKeyForKeyboardEvent = (event) -> else characters.unmodified +exports.MODIFIERS = MODIFIERS + exports.characterForKeyboardEvent = (event) -> event.key unless event.ctrlKey or event.metaKey @@ -194,12 +196,22 @@ exports.calculateSpecificity = calculateSpecificity exports.isBareModifier = (keystroke) -> ENDS_IN_MODIFIER_REGEX.test(keystroke) +exports.isModifierKeyup = (keystroke) -> keystroke.startsWith('^') and ENDS_IN_MODIFIER_REGEX.test(keystroke) + exports.keydownEvent = (key, options) -> return buildKeyboardEvent(key, 'keydown', options) exports.keyupEvent = (key, options) -> return buildKeyboardEvent(key, 'keyup', options) +exports.getModKeys = (keystroke) -> + keys = keystroke.split('-') + mod_keys = [] + for key in keys when MODIFIERS.has(key) + mod_keys.push(key) + mod_keys + + buildKeyboardEvent = (key, eventType, {ctrl, shift, alt, cmd, keyCode, target, location}={}) -> ctrlKey = ctrl ? false altKey = alt ? false diff --git a/src/key-binding.coffee b/src/key-binding.coffee index eaed38d..b18efa0 100644 --- a/src/key-binding.coffee +++ b/src/key-binding.coffee @@ -1,4 +1,4 @@ -{calculateSpecificity} = require './helpers' +{calculateSpecificity, MODIFIERS, getModKeys} = require './helpers' module.exports = class KeyBinding @@ -12,6 +12,7 @@ class KeyBinding @selector = selector.replace(/!important/g, '') @specificity = calculateSpecificity(selector) @index = @constructor.currentIndex++ + @isMatchedModifierKeydownKeyup = null matches: (keystroke) -> multiKeystroke = /\s/.test keystroke @@ -28,3 +29,30 @@ class KeyBinding keyBinding.specificity - @specificity else keyBinding.priority - @priority + + # Returns true iff the binding starts with one or more modifier keydowns and ends + # with the matching set of modifier keyups. + # + # The modifier key order in each must match. Bare modifier keydown + # combinations are not handled specially, e.g. "ctrl ^ctrl" also returns true. + # The keymap manager ignores them, there's no reason to do the additional work + # to identify them again here. + is_matched_modifer_keydown_keyup: -> + # this is likely to get checked repeatedly so we calc it once and cache it + return @isMatchedModifierKeydownKeyup if @isMatchedModifierKeydownKeyup? + + if not @keystrokeArray?.length > 1 + return @isMatchedModifierKeydownKeyup = false + + last_keystroke = @keystrokeArray[@keystrokeArray.length-1] + if not last_keystroke.startsWith('^') + return @isMatchedModifierKeydownKeyup = false + + mod_keys_down = getModKeys(@keystrokeArray[0]) + mod_keys_up = getModKeys(last_keystroke.substring(1)) + if mod_keys_down.length != mod_keys_up.length + return @isMatchedModifierKeydownKeyup = false + for i in [0..mod_keys_down.length-1] + if mod_keys_down[i] != mod_keys_up[i] + return @isMatchedModifierKeydownKeyup = false + return @isMatchedModifierKeydownKeyup = true diff --git a/src/keymap-manager.coffee b/src/keymap-manager.coffee index 13a3c17..1bf24c6 100644 --- a/src/keymap-manager.coffee +++ b/src/keymap-manager.coffee @@ -7,7 +7,7 @@ path = require 'path' {Emitter, Disposable, CompositeDisposable} = require 'event-kit' KeyBinding = require './key-binding' CommandEvent = require './command-event' -{normalizeKeystrokes, keystrokeForKeyboardEvent, isBareModifier, keydownEvent, keyupEvent, characterForKeyboardEvent, keystrokesMatch} = require './helpers' +{normalizeKeystrokes, keystrokeForKeyboardEvent, isBareModifier, keydownEvent, keyupEvent, characterForKeyboardEvent, keystrokesMatch, isModifierKeyup} = require './helpers' Platforms = ['darwin', 'freebsd', 'linux', 'sunos', 'win32'] OtherPlatforms = Platforms.filter (platform) -> platform isnt process.platform @@ -95,6 +95,10 @@ class KeymapManager pendingStateTimeoutHandle: null dvorakQwertyWorkaroundEnabled: false + # Pending matches to bindings that begin with a modifier keydown and and with + # the matching modifier keyup + pendingPartialMatchedModifierKeystrokes: null + ### Section: Construction and Destruction ### @@ -490,7 +494,9 @@ class KeymapManager # # Godspeed. + # keystroke is the atom keybind syntax, e.g. 'ctrl-a' keystroke = @keystrokeForKeyboardEvent(event) + console.log(keystroke) # We dont care about bare modifier keys in the bindings. e.g. `ctrl y` isnt going to work. if event.type is 'keydown' and @queuedKeystrokes.length > 0 and isBareModifier(keystroke) @@ -513,6 +519,15 @@ class KeymapManager dispatchedExactMatch = null partialMatches = @findPartialMatches(partialMatchCandidates, target) + if @pendingPartialMatchedModifierKeystrokes? and isModifierKeyup(keystroke) + for binding in @pendingPartialMatchedModifierKeystrokes + binding_mod_keyups = getModKeys(binding.keystrokeArray[binding.keystrokeArray.length-1]) + keystroke_mod_keyups = getModKeys(keystroke) + if keystroke_mod_keyups.length == 1 and binding_mod_keyups.has(keystroke_mod_keyups[0]) + exactMatchCandidates.push(binding) + # Ian TODO remove from @pendingPartialMatchedModifierKeystrokes + # Ian TODO deal with all the other partial match possibilities + # If any partial match *was* pending but has now failed to match, add it to # the list of bindings to disable so we don't attempt to match it again # during a subsequent event replay by `terminatePendingState`. @@ -525,7 +540,7 @@ class KeymapManager shouldUsePartialMatches = hasPartialMatches # Determine if the current keystrokes match any bindings *exactly*. If we - # do find and exact match, the next step depends on whether we have any + # do find an exact match, the next step depends on whether we have any # partial matches. If we have no partial matches, we dispatch the command # immediately. Otherwise we break and allow ourselves to enter the pending # state with a timeout. @@ -552,7 +567,7 @@ class KeymapManager if hasPartialMatches # When there is a set of bindings like `'ctrl-y', 'ctrl-y ^ctrl'`, # and a `ctrl-y` comes in, this will allow the `ctrl-y` command to be - # dispatched without waiting for any other keystrokes + # dispatched without waiting for any other keystrokes. allPartialMatchesContainKeyupRemainder = true for partialMatch in partialMatches if keydownExactMatchCandidates.indexOf(partialMatch) < 0 @@ -562,6 +577,7 @@ class KeymapManager shouldUsePartialMatches = false if @dispatchCommandEvent(exactMatchCandidate.command, target, event) + console.log('dispatched: ' + exactMatchCandidate.keystrokes) dispatchedExactMatch = exactMatchCandidate eventHandled = true break @@ -700,14 +716,29 @@ class KeymapManager @bindingsToDisable = [] enterPendingState: (pendingPartialMatches, enableTimeout) -> - @cancelPendingState() if @pendingStateTimeoutHandle? + if @pendingStateTimeoutHandle? + @cancelPendingState() + else + @buildPendingPartialMatchedModiferKeystrokes() + @pendingPartialMatches = pendingPartialMatches if enableTimeout @pendingStateTimeoutHandle = setTimeout(@terminatePendingState.bind(this, true), @partialMatchTimeout) - cancelPendingState: -> + buildPendingPartialMatchedModiferKeystrokes: -> + @pendingPartialMatchedModifierKeystrokes = null + for match in @pendingPartialMatches? + if match.is_matched_modifer_keydown_keyup() + @pendingPartialMatchedModifierKeystrokes.push(match) + + cancelPendingState: (modifierKeyupMatched = false) -> clearTimeout(@pendingStateTimeoutHandle) @pendingStateTimeoutHandle = null + + # Ian TODO perf? + # Preserve pending modifier keydown-keyup matches beyond pending state + @buildPendingPartialMatchedModiferKeystrokes() + @pendingPartialMatches = null # This is called by {::handleKeyboardEvent} when no matching bindings are @@ -718,7 +749,7 @@ class KeymapManager # # Note that replaying events has a recursive behavior. Replaying will set # member state (e.g. @queuedKeyboardEvents) just like real events, and will - # likely result in another call this function. The replay process will + # likely result in another call to this function. The replay process will # potentially replay the events (or a subset of events) several times, while # disabling bindings here and there. See any spec that handles multiple # keystrokes failures to match a binding. From 53d9c897b4be3f666b5f1207b6f57c9374f45e74 Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Tue, 18 Oct 2016 15:32:38 -0700 Subject: [PATCH 02/15] fixed style issues from nathansobo's review --- spec/helpers-spec.coffee | 1 + spec/key-binding-spec.coffee | 33 ++++++++++++++++----------------- src/helpers.coffee | 2 +- src/key-binding.coffee | 22 +++++++++++----------- src/keymap-manager.coffee | 9 +++++---- 5 files changed, 34 insertions(+), 33 deletions(-) diff --git a/spec/helpers-spec.coffee b/spec/helpers-spec.coffee index 6bdb47d..fdd787d 100644 --- a/spec/helpers-spec.coffee +++ b/spec/helpers-spec.coffee @@ -70,6 +70,7 @@ describe ".isModifierKeyup(keystroke)", -> assert(isModifierKeyup('^cmd')) assert(isModifierKeyup('^ctrl-shift')) assert(isModifierKeyup('^alt-cmd')) + it "returns false for modifier keydowns", -> assert(!isModifierKeyup('ctrl-x')) assert(!isModifierKeyup('shift-x')) diff --git a/spec/key-binding-spec.coffee b/spec/key-binding-spec.coffee index 61965a4..938714b 100644 --- a/spec/key-binding-spec.coffee +++ b/spec/key-binding-spec.coffee @@ -1,54 +1,53 @@ KeyBinding = require '../src/key-binding' describe "KeyBinding", -> - - describe "is_matched_modifer_keydown_keyup", -> + describe "isMatchedModifierKeydownKeyup", -> describe "returns false when the binding...", -> it "has no modifier keys", -> kb = new KeyBinding('test', 'whatever', 'a', 'body', 0) - assert(not kb.is_matched_modifer_keydown_keyup()) + assert(not kb.isMatchedModifierKeydownKeyup()) it "has no keyups", -> kb = new KeyBinding('test', 'whatever', 'ctrl-a', 'body', 0) - assert(not kb.is_matched_modifer_keydown_keyup()) + assert(not kb.isMatchedModifierKeydownKeyup()) it "is a bare modifier", -> kb = new KeyBinding('test', 'whatever', 'ctrl', 'body', 0) - assert(not kb.is_matched_modifer_keydown_keyup()) + assert(not kb.isMatchedModifierKeydownKeyup()) it "is a bare modifier keyup", -> kb = new KeyBinding('test', 'whatever', '^ctrl', 'body', 0) - assert(not kb.is_matched_modifer_keydown_keyup()) + assert(not kb.isMatchedModifierKeydownKeyup()) it "has mismatched last_keystroke: ctrl ^shift", -> kb = new KeyBinding('test', 'whatever', 'ctrl-a ^shift', 'body', 0) - assert(not kb.is_matched_modifer_keydown_keyup()) + assert(not kb.isMatchedModifierKeydownKeyup()) it "has mismatched last_keystroke: cmd ^alt", -> kb = new KeyBinding('test', 'whatever', 'cmd-a ^alt', 'body', 0) - assert(not kb.is_matched_modifer_keydown_keyup()) + assert(not kb.isMatchedModifierKeydownKeyup()) it "has partially mismatched last_keystroke: ctrl-cmd ^ctrl", -> kb = new KeyBinding('test', 'whatever', 'ctrl-cmd-a ^ctrl', 'body', 0) - assert(not kb.is_matched_modifer_keydown_keyup()) + assert(not kb.isMatchedModifierKeydownKeyup()) it "has partially mismatched last_keystroke: ctrl-cmd ^cmd", -> kb = new KeyBinding('test', 'whatever', 'ctrl-cmd-a ^cmd', 'body', 0) - assert(not kb.is_matched_modifer_keydown_keyup()) + assert(not kb.isMatchedModifierKeydownKeyup()) it "has partially mismatched last_keystroke: ctrl ^ctrl-shift", -> kb = new KeyBinding('test', 'whatever', 'ctrl ^ctrl-shift', 'body', 0) - assert(not kb.is_matched_modifer_keydown_keyup()) + assert(not kb.isMatchedModifierKeydownKeyup()) describe "returns true when the binding...", -> it "has a matched ctrl", -> kb = new KeyBinding('test', 'whatever', 'ctrl-a ^ctrl', 'body', 0) - assert(kb.is_matched_modifer_keydown_keyup()) + assert(kb.isMatchedModifierKeydownKeyup()) it "has a matched shift", -> kb = new KeyBinding('test', 'whatever', 'shift-a ^shift', 'body', 0) - assert(kb.is_matched_modifer_keydown_keyup()) + assert(kb.isMatchedModifierKeydownKeyup()) it "has a matched alt", -> kb = new KeyBinding('test', 'whatever', 'alt-a ^alt', 'body', 0) - assert(kb.is_matched_modifer_keydown_keyup()) + assert(kb.isMatchedModifierKeydownKeyup()) it "has a matched cmd", -> kb = new KeyBinding('test', 'whatever', 'cmd-a ^cmd', 'body', 0) - assert(kb.is_matched_modifer_keydown_keyup()) + assert(kb.isMatchedModifierKeydownKeyup()) it "has a matched ctrl-shift", -> kb = new KeyBinding('test', 'whatever', 'ctrl-shift-a ^ctrl-shift', 'body', 0) - assert(kb.is_matched_modifer_keydown_keyup()) + assert(kb.isMatchedModifierKeydownKeyup()) it "has matched bare last_keystroke", -> kb = new KeyBinding('test', 'whatever', 'ctrl ^ctrl', 'body', 0) - assert(kb.is_matched_modifer_keydown_keyup()) + assert(kb.isMatchedModifierKeydownKeyup()) diff --git a/src/helpers.coffee b/src/helpers.coffee index f79910e..caade0f 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -204,7 +204,7 @@ exports.keydownEvent = (key, options) -> exports.keyupEvent = (key, options) -> return buildKeyboardEvent(key, 'keyup', options) -exports.getModKeys = (keystroke) -> +exports.getModifierKeys = (keystroke) -> keys = keystroke.split('-') mod_keys = [] for key in keys when MODIFIERS.has(key) diff --git a/src/key-binding.coffee b/src/key-binding.coffee index b18efa0..f1dbcc2 100644 --- a/src/key-binding.coffee +++ b/src/key-binding.coffee @@ -1,4 +1,4 @@ -{calculateSpecificity, MODIFIERS, getModKeys} = require './helpers' +{calculateSpecificity, MODIFIERS, getModifierKeys} = require './helpers' module.exports = class KeyBinding @@ -12,7 +12,7 @@ class KeyBinding @selector = selector.replace(/!important/g, '') @specificity = calculateSpecificity(selector) @index = @constructor.currentIndex++ - @isMatchedModifierKeydownKeyup = null + @isMatchedModifierKeydownKeyupCache = null matches: (keystroke) -> multiKeystroke = /\s/.test keystroke @@ -37,22 +37,22 @@ class KeyBinding # combinations are not handled specially, e.g. "ctrl ^ctrl" also returns true. # The keymap manager ignores them, there's no reason to do the additional work # to identify them again here. - is_matched_modifer_keydown_keyup: -> + isMatchedModifierKeydownKeyup: -> # this is likely to get checked repeatedly so we calc it once and cache it - return @isMatchedModifierKeydownKeyup if @isMatchedModifierKeydownKeyup? + return @isMatchedModifierKeydownKeyupCache if @isMatchedModifierKeydownKeyupCache? if not @keystrokeArray?.length > 1 - return @isMatchedModifierKeydownKeyup = false + return @isMatchedModifierKeydownKeyupCache = false last_keystroke = @keystrokeArray[@keystrokeArray.length-1] if not last_keystroke.startsWith('^') - return @isMatchedModifierKeydownKeyup = false + return @isMatchedModifierKeydownKeyupCache = false - mod_keys_down = getModKeys(@keystrokeArray[0]) - mod_keys_up = getModKeys(last_keystroke.substring(1)) + mod_keys_down = getModifierKeys(@keystrokeArray[0]) + mod_keys_up = getModifierKeys(last_keystroke.substring(1)) if mod_keys_down.length != mod_keys_up.length - return @isMatchedModifierKeydownKeyup = false + return @isMatchedModifierKeydownKeyupCache = false for i in [0..mod_keys_down.length-1] if mod_keys_down[i] != mod_keys_up[i] - return @isMatchedModifierKeydownKeyup = false - return @isMatchedModifierKeydownKeyup = true + return @isMatchedModifierKeydownKeyupCache = false + return @isMatchedModifierKeydownKeyupCache = true diff --git a/src/keymap-manager.coffee b/src/keymap-manager.coffee index 1bf24c6..b69a1dc 100644 --- a/src/keymap-manager.coffee +++ b/src/keymap-manager.coffee @@ -95,7 +95,7 @@ class KeymapManager pendingStateTimeoutHandle: null dvorakQwertyWorkaroundEnabled: false - # Pending matches to bindings that begin with a modifier keydown and and with + # Pending matches to bindings that begin with a modifier keydown and end with # the matching modifier keyup pendingPartialMatchedModifierKeystrokes: null @@ -496,6 +496,7 @@ class KeymapManager # keystroke is the atom keybind syntax, e.g. 'ctrl-a' keystroke = @keystrokeForKeyboardEvent(event) + # Ian TODO :fire: console.log(keystroke) # We dont care about bare modifier keys in the bindings. e.g. `ctrl y` isnt going to work. @@ -521,8 +522,8 @@ class KeymapManager if @pendingPartialMatchedModifierKeystrokes? and isModifierKeyup(keystroke) for binding in @pendingPartialMatchedModifierKeystrokes - binding_mod_keyups = getModKeys(binding.keystrokeArray[binding.keystrokeArray.length-1]) - keystroke_mod_keyups = getModKeys(keystroke) + binding_mod_keyups = getModifierKeys(binding.keystrokeArray[binding.keystrokeArray.length-1]) + keystroke_mod_keyups = getModifierKeys(keystroke) if keystroke_mod_keyups.length == 1 and binding_mod_keyups.has(keystroke_mod_keyups[0]) exactMatchCandidates.push(binding) # Ian TODO remove from @pendingPartialMatchedModifierKeystrokes @@ -728,7 +729,7 @@ class KeymapManager buildPendingPartialMatchedModiferKeystrokes: -> @pendingPartialMatchedModifierKeystrokes = null for match in @pendingPartialMatches? - if match.is_matched_modifer_keydown_keyup() + if match.isMatchedModifierKeydownKeyup() @pendingPartialMatchedModifierKeystrokes.push(match) cancelPendingState: (modifierKeyupMatched = false) -> From 1888c707e5b8c22f5c7a6a2f02c18f5e84b37677 Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Tue, 18 Oct 2016 15:47:31 -0700 Subject: [PATCH 03/15] =?UTF-8?q?more=20=F0=9F=90=8D=20=20=E2=96=B6?= =?UTF-8?q?=EF=B8=8F=20=20=F0=9F=90=AB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/key-binding.coffee | 16 ++++++++-------- src/keymap-manager.coffee | 7 ++++--- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/key-binding.coffee b/src/key-binding.coffee index f1dbcc2..d949d2b 100644 --- a/src/key-binding.coffee +++ b/src/key-binding.coffee @@ -44,15 +44,15 @@ class KeyBinding if not @keystrokeArray?.length > 1 return @isMatchedModifierKeydownKeyupCache = false - last_keystroke = @keystrokeArray[@keystrokeArray.length-1] - if not last_keystroke.startsWith('^') + lastKeystroke = @keystrokeArray[@keystrokeArray.length-1] + if not lastKeystroke.startsWith('^') return @isMatchedModifierKeydownKeyupCache = false - mod_keys_down = getModifierKeys(@keystrokeArray[0]) - mod_keys_up = getModifierKeys(last_keystroke.substring(1)) - if mod_keys_down.length != mod_keys_up.length + modifierKeysDown = getModifierKeys(@keystrokeArray[0]) + modifierKeysUp = getModifierKeys(lastKeystroke.substring(1)) + if modifierKeysDown.length != modifierKeysUp.length return @isMatchedModifierKeydownKeyupCache = false - for i in [0..mod_keys_down.length-1] - if mod_keys_down[i] != mod_keys_up[i] + for i in [0..modifierKeysDown.length-1] + if modifierKeysDown[i] != modifierKeysUp[i] return @isMatchedModifierKeydownKeyupCache = false - return @isMatchedModifierKeydownKeyupCache = true + return @isMatchedModifierKeydownKeyup = true diff --git a/src/keymap-manager.coffee b/src/keymap-manager.coffee index b69a1dc..ca2e898 100644 --- a/src/keymap-manager.coffee +++ b/src/keymap-manager.coffee @@ -522,9 +522,9 @@ class KeymapManager if @pendingPartialMatchedModifierKeystrokes? and isModifierKeyup(keystroke) for binding in @pendingPartialMatchedModifierKeystrokes - binding_mod_keyups = getModifierKeys(binding.keystrokeArray[binding.keystrokeArray.length-1]) - keystroke_mod_keyups = getModifierKeys(keystroke) - if keystroke_mod_keyups.length == 1 and binding_mod_keyups.has(keystroke_mod_keyups[0]) + bindingModifierKeyups = getModifierKeys(binding.keystrokeArray[binding.keystrokeArray.length-1]) + keystrokeModifierKeyups = getModifierKeys(keystroke) + if keystrokeModifierKeyups.length == 1 and bindingModifierKeyups.has(keystrokeModifierKeyups[0]) exactMatchCandidates.push(binding) # Ian TODO remove from @pendingPartialMatchedModifierKeystrokes # Ian TODO deal with all the other partial match possibilities @@ -578,6 +578,7 @@ class KeymapManager shouldUsePartialMatches = false if @dispatchCommandEvent(exactMatchCandidate.command, target, event) + # Ian TODO :fire: console.log('dispatched: ' + exactMatchCandidate.keystrokes) dispatchedExactMatch = exactMatchCandidate eventHandled = true From faca3207e6bfdd7e750cd6196cbeda290fb9f7a6 Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Tue, 18 Oct 2016 17:52:19 -0700 Subject: [PATCH 04/15] require only one keyup from the keydown sequence --- spec/key-binding-spec.coffee | 17 ++++++++++------- src/key-binding.coffee | 17 +++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/spec/key-binding-spec.coffee b/spec/key-binding-spec.coffee index 938714b..b4ca903 100644 --- a/spec/key-binding-spec.coffee +++ b/spec/key-binding-spec.coffee @@ -22,13 +22,7 @@ describe "KeyBinding", -> it "has mismatched last_keystroke: cmd ^alt", -> kb = new KeyBinding('test', 'whatever', 'cmd-a ^alt', 'body', 0) assert(not kb.isMatchedModifierKeydownKeyup()) - it "has partially mismatched last_keystroke: ctrl-cmd ^ctrl", -> - kb = new KeyBinding('test', 'whatever', 'ctrl-cmd-a ^ctrl', 'body', 0) - assert(not kb.isMatchedModifierKeydownKeyup()) - it "has partially mismatched last_keystroke: ctrl-cmd ^cmd", -> - kb = new KeyBinding('test', 'whatever', 'ctrl-cmd-a ^cmd', 'body', 0) - assert(not kb.isMatchedModifierKeydownKeyup()) - it "has partially mismatched last_keystroke: ctrl ^ctrl-shift", -> + it "has more keyups than keydowns: ctrl ^ctrl-shift", -> kb = new KeyBinding('test', 'whatever', 'ctrl ^ctrl-shift', 'body', 0) assert(not kb.isMatchedModifierKeydownKeyup()) @@ -51,3 +45,12 @@ describe "KeyBinding", -> it "has matched bare last_keystroke", -> kb = new KeyBinding('test', 'whatever', 'ctrl ^ctrl', 'body', 0) assert(kb.isMatchedModifierKeydownKeyup()) + it "has partially matching last_keystroke: ctrl-cmd ^ctrl", -> + kb = new KeyBinding('test', 'whatever', 'ctrl-cmd-a ^ctrl', 'body', 0) + assert(kb.isMatchedModifierKeydownKeyup()) + it "has partially matching last_keystroke: ctrl-cmd ^cmd", -> + kb = new KeyBinding('test', 'whatever', 'ctrl-cmd-a ^cmd', 'body', 0) + assert(kb.isMatchedModifierKeydownKeyup()) + it "has partially matching last_keystroke: ctrl-shift ^ctrl", -> + kb = new KeyBinding('test', 'whatever', 'ctrl-shift-a ^ctrl', 'body', 0) + assert(kb.isMatchedModifierKeydownKeyup()) diff --git a/src/key-binding.coffee b/src/key-binding.coffee index d949d2b..1f967c9 100644 --- a/src/key-binding.coffee +++ b/src/key-binding.coffee @@ -30,13 +30,12 @@ class KeyBinding else keyBinding.priority - @priority - # Returns true iff the binding starts with one or more modifier keydowns and ends - # with the matching set of modifier keyups. + # Returns true iff the binding starts with one or more modifier keydowns and + # ends with at a subset of matching modifier keyups. # - # The modifier key order in each must match. Bare modifier keydown - # combinations are not handled specially, e.g. "ctrl ^ctrl" also returns true. - # The keymap manager ignores them, there's no reason to do the additional work - # to identify them again here. + # Bare modifier keydown combinations are not handled specially, e.g. + # "ctrl ^ctrl" also returns true. The keymap manager ignores them, there's no + # reason to do the additional work to identify them again here. isMatchedModifierKeydownKeyup: -> # this is likely to get checked repeatedly so we calc it once and cache it return @isMatchedModifierKeydownKeyupCache if @isMatchedModifierKeydownKeyupCache? @@ -50,9 +49,7 @@ class KeyBinding modifierKeysDown = getModifierKeys(@keystrokeArray[0]) modifierKeysUp = getModifierKeys(lastKeystroke.substring(1)) - if modifierKeysDown.length != modifierKeysUp.length - return @isMatchedModifierKeydownKeyupCache = false - for i in [0..modifierKeysDown.length-1] - if modifierKeysDown[i] != modifierKeysUp[i] + for keyup in modifierKeysUp + if modifierKeysDown.indexOf(keyup) < 0 return @isMatchedModifierKeydownKeyupCache = false return @isMatchedModifierKeydownKeyup = true From 5695cc1bdfa44133e06d4e9b8395ca909a05b23d Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Tue, 18 Oct 2016 18:53:04 -0700 Subject: [PATCH 05/15] stop restricting new behavior to modifier keys --- spec/key-binding-spec.coffee | 50 ++++++++++++++++++++---------------- src/key-binding.coffee | 24 ++++++++--------- src/keymap-manager.coffee | 2 +- 3 files changed, 39 insertions(+), 37 deletions(-) diff --git a/spec/key-binding-spec.coffee b/spec/key-binding-spec.coffee index b4ca903..9bc7084 100644 --- a/spec/key-binding-spec.coffee +++ b/spec/key-binding-spec.coffee @@ -1,56 +1,62 @@ KeyBinding = require '../src/key-binding' describe "KeyBinding", -> - describe "isMatchedModifierKeydownKeyup", -> + describe "isMatchedKeydownKeyup", -> describe "returns false when the binding...", -> - it "has no modifier keys", -> - kb = new KeyBinding('test', 'whatever', 'a', 'body', 0) - assert(not kb.isMatchedModifierKeydownKeyup()) it "has no keyups", -> - kb = new KeyBinding('test', 'whatever', 'ctrl-a', 'body', 0) - assert(not kb.isMatchedModifierKeydownKeyup()) - it "is a bare modifier", -> - kb = new KeyBinding('test', 'whatever', 'ctrl', 'body', 0) - assert(not kb.isMatchedModifierKeydownKeyup()) + kb = new KeyBinding('test', 'whatever', 'ctrl-a a 1 2 3', 'body', 0) + assert(not kb.isMatchedKeydownKeyup()) + it "has no keydowns", -> + kb = new KeyBinding('test', 'whatever', '^ctrl ^a', 'body', 0) + assert(not kb.isMatchedKeydownKeyup()) it "is a bare modifier keyup", -> kb = new KeyBinding('test', 'whatever', '^ctrl', 'body', 0) - assert(not kb.isMatchedModifierKeydownKeyup()) + assert(not kb.isMatchedKeydownKeyup()) it "has mismatched last_keystroke: ctrl ^shift", -> kb = new KeyBinding('test', 'whatever', 'ctrl-a ^shift', 'body', 0) - assert(not kb.isMatchedModifierKeydownKeyup()) + assert(not kb.isMatchedKeydownKeyup()) it "has mismatched last_keystroke: cmd ^alt", -> kb = new KeyBinding('test', 'whatever', 'cmd-a ^alt', 'body', 0) - assert(not kb.isMatchedModifierKeydownKeyup()) + assert(not kb.isMatchedKeydownKeyup()) it "has more keyups than keydowns: ctrl ^ctrl-shift", -> kb = new KeyBinding('test', 'whatever', 'ctrl ^ctrl-shift', 'body', 0) - assert(not kb.isMatchedModifierKeydownKeyup()) + assert(not kb.isMatchedKeydownKeyup()) + it "has matching keyups that don't come last", -> + kb = new KeyBinding('test', 'whatever', 'ctrl ^ctrl a', 'body', 0) + assert(not kb.isMatchedKeydownKeyup()) describe "returns true when the binding...", -> it "has a matched ctrl", -> kb = new KeyBinding('test', 'whatever', 'ctrl-a ^ctrl', 'body', 0) - assert(kb.isMatchedModifierKeydownKeyup()) + assert(kb.isMatchedKeydownKeyup()) it "has a matched shift", -> kb = new KeyBinding('test', 'whatever', 'shift-a ^shift', 'body', 0) - assert(kb.isMatchedModifierKeydownKeyup()) + assert(kb.isMatchedKeydownKeyup()) it "has a matched alt", -> kb = new KeyBinding('test', 'whatever', 'alt-a ^alt', 'body', 0) - assert(kb.isMatchedModifierKeydownKeyup()) + assert(kb.isMatchedKeydownKeyup()) it "has a matched cmd", -> kb = new KeyBinding('test', 'whatever', 'cmd-a ^cmd', 'body', 0) - assert(kb.isMatchedModifierKeydownKeyup()) + assert(kb.isMatchedKeydownKeyup()) it "has a matched ctrl-shift", -> kb = new KeyBinding('test', 'whatever', 'ctrl-shift-a ^ctrl-shift', 'body', 0) - assert(kb.isMatchedModifierKeydownKeyup()) + assert(kb.isMatchedKeydownKeyup()) it "has matched bare last_keystroke", -> kb = new KeyBinding('test', 'whatever', 'ctrl ^ctrl', 'body', 0) - assert(kb.isMatchedModifierKeydownKeyup()) + assert(kb.isMatchedKeydownKeyup()) it "has partially matching last_keystroke: ctrl-cmd ^ctrl", -> kb = new KeyBinding('test', 'whatever', 'ctrl-cmd-a ^ctrl', 'body', 0) - assert(kb.isMatchedModifierKeydownKeyup()) + assert(kb.isMatchedKeydownKeyup()) it "has partially matching last_keystroke: ctrl-cmd ^cmd", -> kb = new KeyBinding('test', 'whatever', 'ctrl-cmd-a ^cmd', 'body', 0) - assert(kb.isMatchedModifierKeydownKeyup()) + assert(kb.isMatchedKeydownKeyup()) it "has partially matching last_keystroke: ctrl-shift ^ctrl", -> kb = new KeyBinding('test', 'whatever', 'ctrl-shift-a ^ctrl', 'body', 0) - assert(kb.isMatchedModifierKeydownKeyup()) + assert(kb.isMatchedKeydownKeyup()) + it "has matching non-modifer", -> + kb = new KeyBinding('test', 'whatever', 'a ^a', 'body', 0) + assert(kb.isMatchedKeydownKeyup()) + it "has matching non-modifer with several intermediate keys", -> + kb = new KeyBinding('test', 'whatever', 'a 1 2 3 ^a', 'body', 0) + assert(kb.isMatchedKeydownKeyup()) diff --git a/src/key-binding.coffee b/src/key-binding.coffee index 1f967c9..da8f8cd 100644 --- a/src/key-binding.coffee +++ b/src/key-binding.coffee @@ -12,7 +12,7 @@ class KeyBinding @selector = selector.replace(/!important/g, '') @specificity = calculateSpecificity(selector) @index = @constructor.currentIndex++ - @isMatchedModifierKeydownKeyupCache = null + @isMatchedKeydownKeyupCache = null matches: (keystroke) -> multiKeystroke = /\s/.test keystroke @@ -30,26 +30,22 @@ class KeyBinding else keyBinding.priority - @priority - # Returns true iff the binding starts with one or more modifier keydowns and - # ends with at a subset of matching modifier keyups. - # - # Bare modifier keydown combinations are not handled specially, e.g. - # "ctrl ^ctrl" also returns true. The keymap manager ignores them, there's no - # reason to do the additional work to identify them again here. - isMatchedModifierKeydownKeyup: -> + # Returns true iff the binding starts with one or more keydowns and + # ends with a subset of matching keyups. + isMatchedKeydownKeyup: -> # this is likely to get checked repeatedly so we calc it once and cache it - return @isMatchedModifierKeydownKeyupCache if @isMatchedModifierKeydownKeyupCache? + return @isMatchedKeydownKeyupCache if @isMatchedKeydownKeyupCache? if not @keystrokeArray?.length > 1 - return @isMatchedModifierKeydownKeyupCache = false + return @isMatchedKeydownKeyupCache = false lastKeystroke = @keystrokeArray[@keystrokeArray.length-1] - if not lastKeystroke.startsWith('^') - return @isMatchedModifierKeydownKeyupCache = false + if @keystrokeArray[0].startsWith('^') or not lastKeystroke.startsWith('^') + return @isMatchedKeydownKeyupCache = false modifierKeysDown = getModifierKeys(@keystrokeArray[0]) modifierKeysUp = getModifierKeys(lastKeystroke.substring(1)) for keyup in modifierKeysUp if modifierKeysDown.indexOf(keyup) < 0 - return @isMatchedModifierKeydownKeyupCache = false - return @isMatchedModifierKeydownKeyup = true + return @isMatchedKeydownKeyupCache = false + return isMatchedKeydownKeyupCache = true diff --git a/src/keymap-manager.coffee b/src/keymap-manager.coffee index ca2e898..6cc6040 100644 --- a/src/keymap-manager.coffee +++ b/src/keymap-manager.coffee @@ -730,7 +730,7 @@ class KeymapManager buildPendingPartialMatchedModiferKeystrokes: -> @pendingPartialMatchedModifierKeystrokes = null for match in @pendingPartialMatches? - if match.isMatchedModifierKeydownKeyup() + if match.isMatchedKeydownKeyup() @pendingPartialMatchedModifierKeystrokes.push(match) cancelPendingState: (modifierKeyupMatched = false) -> From 070c406d36e04721724d83e0d5999c8eb3d49d2f Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Wed, 19 Oct 2016 16:59:12 -0700 Subject: [PATCH 06/15] move keystroke matcher to KeyBinding --- spec/helpers-spec.coffee | 30 --------------- spec/key-binding-spec.coffee | 71 ++++++++++++++++++++++++++---------- src/helpers.coffee | 53 --------------------------- src/key-binding.coffee | 60 +++++++++++++++++++++++++++++- src/keymap-manager.coffee | 12 +++--- 5 files changed, 116 insertions(+), 110 deletions(-) diff --git a/spec/helpers-spec.coffee b/spec/helpers-spec.coffee index fdd787d..f5a1e70 100644 --- a/spec/helpers-spec.coffee +++ b/spec/helpers-spec.coffee @@ -32,36 +32,6 @@ describe ".normalizeKeystrokes(keystrokes)", -> assert.equal(normalizeKeystrokes('- '), false) assert.equal(normalizeKeystrokes('a '), false) -describe ".keystrokesMatch(bindingKeystrokes, userKeystrokes)", -> - it "returns 'exact' for exact matches", -> - assert.equal(keystrokesMatch(['ctrl-tab', '^tab', '^ctrl'], ['ctrl-tab', '^tab', '^ctrl']), 'exact') - assert.equal(keystrokesMatch(['ctrl-tab', '^ctrl'], ['ctrl-tab', '^tab', '^ctrl']), 'exact') - assert.equal(keystrokesMatch(['a', 'b', 'c'], ['a', '^a', 'b', '^b', 'c']), 'exact') - assert.equal(keystrokesMatch(['a', 'b', '^b', 'c'], ['a', '^a', 'b', '^b', 'c']), 'exact') - - it "returns false for non-matches", -> - assert.equal(keystrokesMatch(['ctrl-tab', '^tab'], ['ctrl-tab', '^tab', '^ctrl']), false) - assert.equal(keystrokesMatch(['a', 'b', 'c'], ['a', '^a', 'b', '^b', 'c', '^c']), false) - assert.equal(keystrokesMatch(['a', 'b', '^b', 'c'], ['a', '^a', 'b', '^b', 'c', '^c']), false) - - assert.equal(keystrokesMatch(['a'], ['a', '^a', 'b', '^b', 'c', '^c']), false) - assert.equal(keystrokesMatch(['a'], ['a', '^a']), false) - assert.equal(keystrokesMatch(['a', 'c'], ['a', '^a', 'b', '^b', 'c', '^c']), false) - assert.equal(keystrokesMatch(['a', 'b', '^d'], ['a', '^a', 'b', '^b', 'c', '^c']), false) - assert.equal(keystrokesMatch(['a', 'd', '^d'], ['a', '^a', 'b', '^b', 'c', '^c']), false) - assert.equal(keystrokesMatch(['a', 'd', '^d'], ['^c']), false) - - it "returns 'partial' for partial matches", -> - assert.equal(keystrokesMatch(['a', 'b', '^b'], ['a']), 'partial') - assert.equal(keystrokesMatch(['a', 'b', 'c'], ['a']), 'partial') - assert.equal(keystrokesMatch(['a', 'b', 'c'], ['a', '^a']), 'partial') - assert.equal(keystrokesMatch(['a', 'b', 'c'], ['a', '^a', 'b']), 'partial') - assert.equal(keystrokesMatch(['a', 'b', 'c'], ['a', '^a', 'b', '^b']), 'partial') - assert.equal(keystrokesMatch(['a', 'b', 'c'], ['a', '^a', 'd', '^d']), false) - - it "returns 'keydownExact' for bindings that match and contain a remainder of only keyup events", -> - assert.equal(keystrokesMatch(['a', 'b', '^b'], ['a', 'b']), 'keydownExact') - describe ".isModifierKeyup(keystroke)", -> it "returns true for single modifier keyups", -> assert(isModifierKeyup('^ctrl')) diff --git a/spec/key-binding-spec.coffee b/spec/key-binding-spec.coffee index 9bc7084..69e7f1d 100644 --- a/spec/key-binding-spec.coffee +++ b/spec/key-binding-spec.coffee @@ -1,62 +1,95 @@ -KeyBinding = require '../src/key-binding' +{KeyBinding} = require '../src/key-binding' describe "KeyBinding", -> describe "isMatchedKeydownKeyup", -> describe "returns false when the binding...", -> it "has no keyups", -> - kb = new KeyBinding('test', 'whatever', 'ctrl-a a 1 2 3', 'body', 0) + kb = keyBindingArgHelper('ctrl-a a 1 2 3') assert(not kb.isMatchedKeydownKeyup()) it "has no keydowns", -> - kb = new KeyBinding('test', 'whatever', '^ctrl ^a', 'body', 0) + kb = keyBindingArgHelper('^ctrl ^a') assert(not kb.isMatchedKeydownKeyup()) it "is a bare modifier keyup", -> - kb = new KeyBinding('test', 'whatever', '^ctrl', 'body', 0) + kb = keyBindingArgHelper('^ctrl') assert(not kb.isMatchedKeydownKeyup()) it "has mismatched last_keystroke: ctrl ^shift", -> - kb = new KeyBinding('test', 'whatever', 'ctrl-a ^shift', 'body', 0) + kb = keyBindingArgHelper('ctrl-a ^shift') assert(not kb.isMatchedKeydownKeyup()) it "has mismatched last_keystroke: cmd ^alt", -> - kb = new KeyBinding('test', 'whatever', 'cmd-a ^alt', 'body', 0) + kb = keyBindingArgHelper('cmd-a ^alt') assert(not kb.isMatchedKeydownKeyup()) it "has more keyups than keydowns: ctrl ^ctrl-shift", -> - kb = new KeyBinding('test', 'whatever', 'ctrl ^ctrl-shift', 'body', 0) + kb = keyBindingArgHelper('ctrl ^ctrl-shift') assert(not kb.isMatchedKeydownKeyup()) it "has matching keyups that don't come last", -> - kb = new KeyBinding('test', 'whatever', 'ctrl ^ctrl a', 'body', 0) + kb = keyBindingArgHelper('ctrl ^ctrl a') assert(not kb.isMatchedKeydownKeyup()) describe "returns true when the binding...", -> it "has a matched ctrl", -> - kb = new KeyBinding('test', 'whatever', 'ctrl-a ^ctrl', 'body', 0) + kb = keyBindingArgHelper('ctrl-a ^ctrl') assert(kb.isMatchedKeydownKeyup()) it "has a matched shift", -> - kb = new KeyBinding('test', 'whatever', 'shift-a ^shift', 'body', 0) + kb = keyBindingArgHelper('shift-a ^shift') assert(kb.isMatchedKeydownKeyup()) it "has a matched alt", -> - kb = new KeyBinding('test', 'whatever', 'alt-a ^alt', 'body', 0) + kb = keyBindingArgHelper('alt-a ^alt') assert(kb.isMatchedKeydownKeyup()) it "has a matched cmd", -> - kb = new KeyBinding('test', 'whatever', 'cmd-a ^cmd', 'body', 0) + kb = keyBindingArgHelper('cmd-a ^cmd') assert(kb.isMatchedKeydownKeyup()) it "has a matched ctrl-shift", -> - kb = new KeyBinding('test', 'whatever', 'ctrl-shift-a ^ctrl-shift', 'body', 0) + kb = keyBindingArgHelper('ctrl-shift-a ^ctrl-shift') assert(kb.isMatchedKeydownKeyup()) it "has matched bare last_keystroke", -> - kb = new KeyBinding('test', 'whatever', 'ctrl ^ctrl', 'body', 0) + kb = keyBindingArgHelper('ctrl ^ctrl') assert(kb.isMatchedKeydownKeyup()) it "has partially matching last_keystroke: ctrl-cmd ^ctrl", -> - kb = new KeyBinding('test', 'whatever', 'ctrl-cmd-a ^ctrl', 'body', 0) + kb = keyBindingArgHelper('ctrl-cmd-a ^ctrl') assert(kb.isMatchedKeydownKeyup()) it "has partially matching last_keystroke: ctrl-cmd ^cmd", -> - kb = new KeyBinding('test', 'whatever', 'ctrl-cmd-a ^cmd', 'body', 0) + kb = keyBindingArgHelper('ctrl-cmd-a ^cmd') assert(kb.isMatchedKeydownKeyup()) it "has partially matching last_keystroke: ctrl-shift ^ctrl", -> - kb = new KeyBinding('test', 'whatever', 'ctrl-shift-a ^ctrl', 'body', 0) + kb = keyBindingArgHelper('ctrl-shift-a ^ctrl') assert(kb.isMatchedKeydownKeyup()) it "has matching non-modifer", -> - kb = new KeyBinding('test', 'whatever', 'a ^a', 'body', 0) + kb = keyBindingArgHelper('a ^a') assert(kb.isMatchedKeydownKeyup()) it "has matching non-modifer with several intermediate keys", -> - kb = new KeyBinding('test', 'whatever', 'a 1 2 3 ^a', 'body', 0) + kb = keyBindingArgHelper('a 1 2 3 ^a') assert(kb.isMatchedKeydownKeyup()) + + describe ".matchesKeystrokes(userKeystrokes)", -> + it "returns 'exact' for exact matches", -> + assert.equal(keyBindingArgHelper('ctrl-tab ^tab ^ctrl').matchesKeystrokes(['ctrl-tab', '^tab', '^ctrl']), 'exact') + assert.equal(keyBindingArgHelper('ctrl-tab ^ctrl').matchesKeystrokes(['ctrl-tab', '^tab', '^ctrl']), 'exact') + assert.equal(keyBindingArgHelper('a b c').matchesKeystrokes(['a', '^a', 'b', '^b', 'c']), 'exact') + assert.equal(keyBindingArgHelper('a b ^b c').matchesKeystrokes(['a', '^a', 'b', '^b', 'c']), 'exact') + + it "returns false for non-matches", -> + assert.equal(keyBindingArgHelper('ctrl-tab ^tab').matchesKeystrokes(['ctrl-tab', '^tab', '^ctrl']), false) + assert.equal(keyBindingArgHelper('a b c').matchesKeystrokes(['a', '^a', 'b', '^b', 'c', '^c']), false) + assert.equal(keyBindingArgHelper('a b ^b c').matchesKeystrokes(['a', '^a', 'b', '^b', 'c', '^c']), false) + + assert.equal(keyBindingArgHelper('a').matchesKeystrokes(['a', '^a', 'b', '^b', 'c', '^c']), false) + assert.equal(keyBindingArgHelper('a').matchesKeystrokes(['a', '^a']), false) + assert.equal(keyBindingArgHelper('a c').matchesKeystrokes(['a', '^a', 'b', '^b', 'c', '^c']), false) + assert.equal(keyBindingArgHelper('a b ^d').matchesKeystrokes(['a', '^a', 'b', '^b', 'c', '^c']), false) + assert.equal(keyBindingArgHelper('a d ^d').matchesKeystrokes(['a', '^a', 'b', '^b', 'c', '^c']), false) + assert.equal(keyBindingArgHelper('a d ^d').matchesKeystrokes(['^c']), false) + + it "returns 'partial' for partial matches", -> + assert.equal(keyBindingArgHelper('a b ^b').matchesKeystrokes(['a']), 'partial') + assert.equal(keyBindingArgHelper('a b c').matchesKeystrokes(['a']), 'partial') + assert.equal(keyBindingArgHelper('a b c').matchesKeystrokes(['a', '^a']), 'partial') + assert.equal(keyBindingArgHelper('a b c').matchesKeystrokes(['a', '^a', 'b']), 'partial') + assert.equal(keyBindingArgHelper('a b c').matchesKeystrokes(['a', '^a', 'b', '^b']), 'partial') + assert.equal(keyBindingArgHelper('a b c').matchesKeystrokes(['a', '^a', 'd', '^d']), false) + + it "returns 'keydownExact' for bindings that match and contain a remainder of only keyup events", -> + assert.equal(keyBindingArgHelper('a b ^b').matchesKeystrokes(['a', 'b']), 'keydownExact') + +keyBindingArgHelper = (binding) -> + return new KeyBinding('test', 'test', binding, 'body', 0) diff --git a/src/helpers.coffee b/src/helpers.coffee index caade0f..609c3dd 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -12,11 +12,6 @@ NON_CHARACTER_KEY_NAMES_BY_KEYBOARD_EVENT_KEY = { 'ArrowLeft': 'left', 'ArrowRight': 'right' } -MATCH_TYPES = { - EXACT: 'exact' - KEYDOWN_EXACT: 'keydownExact' - PARTIAL: 'partial' -} isASCIICharacter = (character) -> character? and character.length is 1 and character.charCodeAt(0) <= 127 @@ -228,51 +223,3 @@ buildKeyboardEvent = (key, eventType, {ctrl, shift, alt, cmd, keyCode, target, l Object.defineProperty(event, 'target', get: -> target) Object.defineProperty(event, 'path', get: -> [target]) event - -# bindingKeystrokes and userKeystrokes are arrays of keystrokes -# e.g. ['ctrl-y', 'ctrl-x', '^x'] -exports.keystrokesMatch = (bindingKeystrokes, userKeystrokes) -> - userKeystrokeIndex = -1 - userKeystrokesHasKeydownEvent = false - matchesNextUserKeystroke = (bindingKeystroke) -> - while userKeystrokeIndex < userKeystrokes.length - 1 - userKeystrokeIndex += 1 - userKeystroke = userKeystrokes[userKeystrokeIndex] - isKeydownEvent = not userKeystroke.startsWith('^') - userKeystrokesHasKeydownEvent = true if isKeydownEvent - if bindingKeystroke is userKeystroke - return true - else if isKeydownEvent - return false - null - - isPartialMatch = false - bindingRemainderContainsOnlyKeyups = true - bindingKeystrokeIndex = 0 - for bindingKeystroke in bindingKeystrokes - unless isPartialMatch - doesMatch = matchesNextUserKeystroke(bindingKeystroke) - if doesMatch is false - return false - else if doesMatch is null - # Make sure userKeystrokes with only keyup events doesn't match everything - if userKeystrokesHasKeydownEvent - isPartialMatch = true - else - return false - - if isPartialMatch - bindingRemainderContainsOnlyKeyups = false unless bindingKeystroke.startsWith('^') - - # Bindings that match the beginning of the user's keystrokes are not a match. - # e.g. This is not a match. It would have been a match on the previous keystroke: - # bindingKeystrokes = ['ctrl-tab', '^tab'] - # userKeystrokes = ['ctrl-tab', '^tab', '^ctrl'] - return false if userKeystrokeIndex < userKeystrokes.length - 1 - - if isPartialMatch and bindingRemainderContainsOnlyKeyups - MATCH_TYPES.KEYDOWN_EXACT - else if isPartialMatch - MATCH_TYPES.PARTIAL - else - MATCH_TYPES.EXACT diff --git a/src/key-binding.coffee b/src/key-binding.coffee index da8f8cd..00d932a 100644 --- a/src/key-binding.coffee +++ b/src/key-binding.coffee @@ -1,6 +1,14 @@ -{calculateSpecificity, MODIFIERS, getModifierKeys} = require './helpers' +{calculateSpecificity, MODIFIERS, getModifierKeys, MATCH_TYPES} = require './helpers' -module.exports = + +MATCH_TYPES = { + EXACT: 'exact' + KEYDOWN_EXACT: 'keydownExact' + PARTIAL: 'partial' +} +module.exports.MATCH_TYPES = MATCH_TYPES + +module.exports.KeyBinding = class KeyBinding @currentIndex: 1 @@ -49,3 +57,51 @@ class KeyBinding if modifierKeysDown.indexOf(keyup) < 0 return @isMatchedKeydownKeyupCache = false return isMatchedKeydownKeyupCache = true + + # userKeystrokes is an array of keystrokes e.g. + # ['ctrl-y', 'ctrl-x', '^x'] + matchesKeystrokes: (userKeystrokes) -> + userKeystrokeIndex = -1 + userKeystrokesHasKeydownEvent = false + matchesNextUserKeystroke = (bindingKeystroke) -> + while userKeystrokeIndex < userKeystrokes.length - 1 + userKeystrokeIndex += 1 + userKeystroke = userKeystrokes[userKeystrokeIndex] + isKeydownEvent = not userKeystroke.startsWith('^') + userKeystrokesHasKeydownEvent = true if isKeydownEvent + if bindingKeystroke is userKeystroke + return true + else if isKeydownEvent + return false + null + + isPartialMatch = false + bindingRemainderContainsOnlyKeyups = true + bindingKeystrokeIndex = 0 + for bindingKeystroke in @keystrokeArray + unless isPartialMatch + doesMatch = matchesNextUserKeystroke(bindingKeystroke) + if doesMatch is false + return false + else if doesMatch is null + # Make sure userKeystrokes with only keyup events don't match everything + if userKeystrokesHasKeydownEvent + isPartialMatch = true + else + return false + + if isPartialMatch + bindingRemainderContainsOnlyKeyups = false unless bindingKeystroke.startsWith('^') + + # Bindings that match the beginning of the user's keystrokes are not a match. + # e.g. This is not a match. It would have been a match on the previous keystroke: + # bindingKeystrokes = ['ctrl-tab', '^tab'] + # userKeystrokes = ['ctrl-tab', '^tab', '^ctrl'] + return false if userKeystrokeIndex < userKeystrokes.length - 1 + + if isPartialMatch and bindingRemainderContainsOnlyKeyups + MATCH_TYPES.KEYDOWN_EXACT + else if isPartialMatch + MATCH_TYPES.PARTIAL + else + MATCH_TYPES.EXACT diff --git a/src/keymap-manager.coffee b/src/keymap-manager.coffee index 6cc6040..95b9384 100644 --- a/src/keymap-manager.coffee +++ b/src/keymap-manager.coffee @@ -5,7 +5,7 @@ fs = require 'fs-plus' path = require 'path' {File} = require 'pathwatcher' {Emitter, Disposable, CompositeDisposable} = require 'event-kit' -KeyBinding = require './key-binding' +{KeyBinding, MATCH_TYPES} = require './key-binding' CommandEvent = require './command-event' {normalizeKeystrokes, keystrokeForKeyboardEvent, isBareModifier, keydownEvent, keyupEvent, characterForKeyboardEvent, keystrokesMatch, isModifierKeyup} = require './helpers' @@ -672,12 +672,12 @@ class KeymapManager disabledBindingSet = new Set(disabledBindings) for binding in @keyBindings when not disabledBindingSet.has(binding) - doesMatch = keystrokesMatch(binding.keystrokeArray, keystrokeArray) - if doesMatch is 'exact' + doesMatch = binding.matchesKeystrokes(keystrokeArray) + if doesMatch is MATCH_TYPES.EXACT exactMatchCandidates.push(binding) - else if doesMatch is 'partial' + else if doesMatch is MATCH_TYPES.PARTIAL partialMatchCandidates.push(binding) - else if doesMatch is 'keydownExact' + else if doesMatch is MATCH_TYPES.KEYDOWN_EXACT partialMatchCandidates.push(binding) keydownExactMatchCandidates.push(binding) {partialMatchCandidates, keydownExactMatchCandidates, exactMatchCandidates} @@ -730,7 +730,7 @@ class KeymapManager buildPendingPartialMatchedModiferKeystrokes: -> @pendingPartialMatchedModifierKeystrokes = null for match in @pendingPartialMatches? - if match.isMatchedKeydownKeyup() + if match.isMatchedModifierKeydownKeyup() @pendingPartialMatchedModifierKeystrokes.push(match) cancelPendingState: (modifierKeyupMatched = false) -> From c3bcde2732b4f633bc0f3fb7d8bd2c3eaf2a5c4a Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Thu, 20 Oct 2016 09:48:15 -0700 Subject: [PATCH 07/15] start unifying with keydownExact matches. use MATCH_TYPES everywhere. --- spec/key-binding-spec.coffee | 6 +++--- src/key-binding.coffee | 11 +++++------ src/keymap-manager.coffee | 37 +++++++++++------------------------- 3 files changed, 19 insertions(+), 35 deletions(-) diff --git a/spec/key-binding-spec.coffee b/spec/key-binding-spec.coffee index 69e7f1d..051e8ce 100644 --- a/spec/key-binding-spec.coffee +++ b/spec/key-binding-spec.coffee @@ -1,4 +1,4 @@ -{KeyBinding} = require '../src/key-binding' +{KeyBinding, MATCH_TYPES} = require '../src/key-binding' describe "KeyBinding", -> describe "isMatchedKeydownKeyup", -> @@ -88,8 +88,8 @@ describe "KeyBinding", -> assert.equal(keyBindingArgHelper('a b c').matchesKeystrokes(['a', '^a', 'b', '^b']), 'partial') assert.equal(keyBindingArgHelper('a b c').matchesKeystrokes(['a', '^a', 'd', '^d']), false) - it "returns 'keydownExact' for bindings that match and contain a remainder of only keyup events", -> - assert.equal(keyBindingArgHelper('a b ^b').matchesKeystrokes(['a', 'b']), 'keydownExact') + it "returns MATCH_TYPES.PENDING_KEYUP for bindings that match and contain a remainder of only keyup events", -> + assert.equal(keyBindingArgHelper('a b ^b').matchesKeystrokes(['a', 'b']), MATCH_TYPES.PENDING_KEYUP) keyBindingArgHelper = (binding) -> return new KeyBinding('test', 'test', binding, 'body', 0) diff --git a/src/key-binding.coffee b/src/key-binding.coffee index 00d932a..e4e3a7a 100644 --- a/src/key-binding.coffee +++ b/src/key-binding.coffee @@ -1,10 +1,9 @@ -{calculateSpecificity, MODIFIERS, getModifierKeys, MATCH_TYPES} = require './helpers' - +{calculateSpecificity, MODIFIERS, getModifierKeys} = require './helpers' MATCH_TYPES = { EXACT: 'exact' - KEYDOWN_EXACT: 'keydownExact' PARTIAL: 'partial' + PENDING_KEYUP: 'pendingKeyup' } module.exports.MATCH_TYPES = MATCH_TYPES @@ -56,7 +55,7 @@ class KeyBinding for keyup in modifierKeysUp if modifierKeysDown.indexOf(keyup) < 0 return @isMatchedKeydownKeyupCache = false - return isMatchedKeydownKeyupCache = true + return @isMatchedKeydownKeyupCache = true # userKeystrokes is an array of keystrokes e.g. # ['ctrl-y', 'ctrl-x', '^x'] @@ -99,8 +98,8 @@ class KeyBinding # userKeystrokes = ['ctrl-tab', '^tab', '^ctrl'] return false if userKeystrokeIndex < userKeystrokes.length - 1 - if isPartialMatch and bindingRemainderContainsOnlyKeyups - MATCH_TYPES.KEYDOWN_EXACT + if isPartialMatch and bindingRemainderContainsOnlyKeyups and @isMatchedKeydownKeyup() + MATCH_TYPES.PENDING_KEYUP else if isPartialMatch MATCH_TYPES.PARTIAL else diff --git a/src/keymap-manager.coffee b/src/keymap-manager.coffee index 95b9384..3e3c351 100644 --- a/src/keymap-manager.coffee +++ b/src/keymap-manager.coffee @@ -95,9 +95,9 @@ class KeymapManager pendingStateTimeoutHandle: null dvorakQwertyWorkaroundEnabled: false - # Pending matches to bindings that begin with a modifier keydown and end with - # the matching modifier keyup - pendingPartialMatchedModifierKeystrokes: null + # Pending matches to bindings that begin with keydowns and end with a subset + # of matching keyups + pendingKeyupMatches: null ### Section: Construction and Destruction @@ -516,7 +516,7 @@ class KeymapManager # First screen for any bindings that match the current keystrokes, # regardless of their current selector. Matching strings is cheaper than # matching selectors. - {partialMatchCandidates, keydownExactMatchCandidates, exactMatchCandidates} = @findMatchCandidates(@queuedKeystrokes, disabledBindings) + {partialMatchCandidates, pendingKeyupMatchCandidates, exactMatchCandidates} = @findMatchCandidates(@queuedKeystrokes, disabledBindings) dispatchedExactMatch = null partialMatches = @findPartialMatches(partialMatchCandidates, target) @@ -571,7 +571,7 @@ class KeymapManager # dispatched without waiting for any other keystrokes. allPartialMatchesContainKeyupRemainder = true for partialMatch in partialMatches - if keydownExactMatchCandidates.indexOf(partialMatch) < 0 + if pendingKeyupMatchCandidates.indexOf(partialMatch) < 0 allPartialMatchesContainKeyupRemainder = false break if allPartialMatchesContainKeyupRemainder is false else @@ -668,7 +668,7 @@ class KeymapManager findMatchCandidates: (keystrokeArray, disabledBindings) -> partialMatchCandidates = [] exactMatchCandidates = [] - keydownExactMatchCandidates = [] + pendingKeyupMatchCandidates = [] disabledBindingSet = new Set(disabledBindings) for binding in @keyBindings when not disabledBindingSet.has(binding) @@ -677,10 +677,10 @@ class KeymapManager exactMatchCandidates.push(binding) else if doesMatch is MATCH_TYPES.PARTIAL partialMatchCandidates.push(binding) - else if doesMatch is MATCH_TYPES.KEYDOWN_EXACT + else if doesMatch is MATCH_TYPES.PENDING_KEYUP partialMatchCandidates.push(binding) - keydownExactMatchCandidates.push(binding) - {partialMatchCandidates, keydownExactMatchCandidates, exactMatchCandidates} + pendingKeyupMatchCandidates.push(binding) + {partialMatchCandidates, pendingKeyupMatchCandidates, exactMatchCandidates} # Determine which of the given bindings have selectors matching the target or # one of its ancestors. This is used by {::handleKeyboardEvent} to determine @@ -718,29 +718,14 @@ class KeymapManager @bindingsToDisable = [] enterPendingState: (pendingPartialMatches, enableTimeout) -> - if @pendingStateTimeoutHandle? - @cancelPendingState() - else - @buildPendingPartialMatchedModiferKeystrokes() - + @cancelPendingState() if @pendingStateTimeoutHandle? @pendingPartialMatches = pendingPartialMatches if enableTimeout @pendingStateTimeoutHandle = setTimeout(@terminatePendingState.bind(this, true), @partialMatchTimeout) - buildPendingPartialMatchedModiferKeystrokes: -> - @pendingPartialMatchedModifierKeystrokes = null - for match in @pendingPartialMatches? - if match.isMatchedModifierKeydownKeyup() - @pendingPartialMatchedModifierKeystrokes.push(match) - - cancelPendingState: (modifierKeyupMatched = false) -> + cancelPendingState: -> clearTimeout(@pendingStateTimeoutHandle) @pendingStateTimeoutHandle = null - - # Ian TODO perf? - # Preserve pending modifier keydown-keyup matches beyond pending state - @buildPendingPartialMatchedModiferKeystrokes() - @pendingPartialMatches = null # This is called by {::handleKeyboardEvent} when no matching bindings are From f91f755de31819d472e98af065ed62684bdd8abe Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Thu, 20 Oct 2016 12:56:07 -0700 Subject: [PATCH 08/15] reset for simpler new plan. tests all pass. --- spec/key-binding-spec.coffee | 60 ------------------------------------ src/helpers.coffee | 12 +++++--- src/key-binding.coffee | 24 ++------------- src/keymap-manager.coffee | 19 ++++++------ 4 files changed, 18 insertions(+), 97 deletions(-) diff --git a/spec/key-binding-spec.coffee b/spec/key-binding-spec.coffee index 051e8ce..7512dab 100644 --- a/spec/key-binding-spec.coffee +++ b/spec/key-binding-spec.coffee @@ -1,66 +1,6 @@ {KeyBinding, MATCH_TYPES} = require '../src/key-binding' describe "KeyBinding", -> - describe "isMatchedKeydownKeyup", -> - - describe "returns false when the binding...", -> - it "has no keyups", -> - kb = keyBindingArgHelper('ctrl-a a 1 2 3') - assert(not kb.isMatchedKeydownKeyup()) - it "has no keydowns", -> - kb = keyBindingArgHelper('^ctrl ^a') - assert(not kb.isMatchedKeydownKeyup()) - it "is a bare modifier keyup", -> - kb = keyBindingArgHelper('^ctrl') - assert(not kb.isMatchedKeydownKeyup()) - it "has mismatched last_keystroke: ctrl ^shift", -> - kb = keyBindingArgHelper('ctrl-a ^shift') - assert(not kb.isMatchedKeydownKeyup()) - it "has mismatched last_keystroke: cmd ^alt", -> - kb = keyBindingArgHelper('cmd-a ^alt') - assert(not kb.isMatchedKeydownKeyup()) - it "has more keyups than keydowns: ctrl ^ctrl-shift", -> - kb = keyBindingArgHelper('ctrl ^ctrl-shift') - assert(not kb.isMatchedKeydownKeyup()) - it "has matching keyups that don't come last", -> - kb = keyBindingArgHelper('ctrl ^ctrl a') - assert(not kb.isMatchedKeydownKeyup()) - - describe "returns true when the binding...", -> - it "has a matched ctrl", -> - kb = keyBindingArgHelper('ctrl-a ^ctrl') - assert(kb.isMatchedKeydownKeyup()) - it "has a matched shift", -> - kb = keyBindingArgHelper('shift-a ^shift') - assert(kb.isMatchedKeydownKeyup()) - it "has a matched alt", -> - kb = keyBindingArgHelper('alt-a ^alt') - assert(kb.isMatchedKeydownKeyup()) - it "has a matched cmd", -> - kb = keyBindingArgHelper('cmd-a ^cmd') - assert(kb.isMatchedKeydownKeyup()) - it "has a matched ctrl-shift", -> - kb = keyBindingArgHelper('ctrl-shift-a ^ctrl-shift') - assert(kb.isMatchedKeydownKeyup()) - it "has matched bare last_keystroke", -> - kb = keyBindingArgHelper('ctrl ^ctrl') - assert(kb.isMatchedKeydownKeyup()) - it "has partially matching last_keystroke: ctrl-cmd ^ctrl", -> - kb = keyBindingArgHelper('ctrl-cmd-a ^ctrl') - assert(kb.isMatchedKeydownKeyup()) - it "has partially matching last_keystroke: ctrl-cmd ^cmd", -> - kb = keyBindingArgHelper('ctrl-cmd-a ^cmd') - assert(kb.isMatchedKeydownKeyup()) - it "has partially matching last_keystroke: ctrl-shift ^ctrl", -> - kb = keyBindingArgHelper('ctrl-shift-a ^ctrl') - assert(kb.isMatchedKeydownKeyup()) - it "has matching non-modifer", -> - kb = keyBindingArgHelper('a ^a') - assert(kb.isMatchedKeydownKeyup()) - it "has matching non-modifer with several intermediate keys", -> - kb = keyBindingArgHelper('a 1 2 3 ^a') - assert(kb.isMatchedKeydownKeyup()) - describe ".matchesKeystrokes(userKeystrokes)", -> it "returns 'exact' for exact matches", -> assert.equal(keyBindingArgHelper('ctrl-tab ^tab ^ctrl').matchesKeystrokes(['ctrl-tab', '^tab', '^ctrl']), 'exact') diff --git a/src/helpers.coffee b/src/helpers.coffee index 609c3dd..be33ab5 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -40,7 +40,7 @@ exports.normalizeKeystrokes = (keystrokes) -> normalizedKeystrokes.join(' ') normalizeKeystroke = (keystroke) -> - if isKeyup = keystroke.startsWith('^') + if keyup = isKeyup(keystroke) keystroke = keystroke.slice(1) keys = parseKeystroke(keystroke) return false unless keys @@ -58,7 +58,7 @@ normalizeKeystroke = (keystroke) -> else return false - if isKeyup + if keyup primaryKey = primaryKey.toLowerCase() if primaryKey? else modifiers.add('shift') if isUpperCaseCharacter(primaryKey) @@ -66,14 +66,14 @@ normalizeKeystroke = (keystroke) -> primaryKey = primaryKey.toUpperCase() keystroke = [] - if not isKeyup or (isKeyup and not primaryKey?) + if not keyup or (keyup and not primaryKey?) keystroke.push('ctrl') if modifiers.has('ctrl') keystroke.push('alt') if modifiers.has('alt') keystroke.push('shift') if modifiers.has('shift') keystroke.push('cmd') if modifiers.has('cmd') keystroke.push(primaryKey) if primaryKey? keystroke = keystroke.join('-') - keystroke = "^#{keystroke}" if isKeyup + keystroke = "^#{keystroke}" if keyup keystroke parseKeystroke = (keystroke) -> @@ -191,7 +191,9 @@ exports.calculateSpecificity = calculateSpecificity exports.isBareModifier = (keystroke) -> ENDS_IN_MODIFIER_REGEX.test(keystroke) -exports.isModifierKeyup = (keystroke) -> keystroke.startsWith('^') and ENDS_IN_MODIFIER_REGEX.test(keystroke) +exports.isModifierKeyup = (keystroke) -> isKeyup(keystroke) and ENDS_IN_MODIFIER_REGEX.test(keystroke) + +exports.isKeyup = isKeyup = (keystroke) -> keystroke.startsWith('^') exports.keydownEvent = (key, options) -> return buildKeyboardEvent(key, 'keydown', options) diff --git a/src/key-binding.coffee b/src/key-binding.coffee index e4e3a7a..ed14598 100644 --- a/src/key-binding.coffee +++ b/src/key-binding.coffee @@ -1,4 +1,4 @@ -{calculateSpecificity, MODIFIERS, getModifierKeys} = require './helpers' +{calculateSpecificity, MODIFIERS, isKeyup} = require './helpers' MATCH_TYPES = { EXACT: 'exact' @@ -37,26 +37,6 @@ class KeyBinding else keyBinding.priority - @priority - # Returns true iff the binding starts with one or more keydowns and - # ends with a subset of matching keyups. - isMatchedKeydownKeyup: -> - # this is likely to get checked repeatedly so we calc it once and cache it - return @isMatchedKeydownKeyupCache if @isMatchedKeydownKeyupCache? - - if not @keystrokeArray?.length > 1 - return @isMatchedKeydownKeyupCache = false - - lastKeystroke = @keystrokeArray[@keystrokeArray.length-1] - if @keystrokeArray[0].startsWith('^') or not lastKeystroke.startsWith('^') - return @isMatchedKeydownKeyupCache = false - - modifierKeysDown = getModifierKeys(@keystrokeArray[0]) - modifierKeysUp = getModifierKeys(lastKeystroke.substring(1)) - for keyup in modifierKeysUp - if modifierKeysDown.indexOf(keyup) < 0 - return @isMatchedKeydownKeyupCache = false - return @isMatchedKeydownKeyupCache = true - # userKeystrokes is an array of keystrokes e.g. # ['ctrl-y', 'ctrl-x', '^x'] matchesKeystrokes: (userKeystrokes) -> @@ -98,7 +78,7 @@ class KeyBinding # userKeystrokes = ['ctrl-tab', '^tab', '^ctrl'] return false if userKeystrokeIndex < userKeystrokes.length - 1 - if isPartialMatch and bindingRemainderContainsOnlyKeyups and @isMatchedKeydownKeyup() + if isPartialMatch and bindingRemainderContainsOnlyKeyups MATCH_TYPES.PENDING_KEYUP else if isPartialMatch MATCH_TYPES.PARTIAL diff --git a/src/keymap-manager.coffee b/src/keymap-manager.coffee index 3e3c351..f80f0cb 100644 --- a/src/keymap-manager.coffee +++ b/src/keymap-manager.coffee @@ -7,7 +7,7 @@ path = require 'path' {Emitter, Disposable, CompositeDisposable} = require 'event-kit' {KeyBinding, MATCH_TYPES} = require './key-binding' CommandEvent = require './command-event' -{normalizeKeystrokes, keystrokeForKeyboardEvent, isBareModifier, keydownEvent, keyupEvent, characterForKeyboardEvent, keystrokesMatch, isModifierKeyup} = require './helpers' +{normalizeKeystrokes, keystrokeForKeyboardEvent, isBareModifier, keydownEvent, keyupEvent, characterForKeyboardEvent, keystrokesMatch, isKeyup} = require './helpers' Platforms = ['darwin', 'freebsd', 'linux', 'sunos', 'win32'] OtherPlatforms = Platforms.filter (platform) -> platform isnt process.platform @@ -520,15 +520,6 @@ class KeymapManager dispatchedExactMatch = null partialMatches = @findPartialMatches(partialMatchCandidates, target) - if @pendingPartialMatchedModifierKeystrokes? and isModifierKeyup(keystroke) - for binding in @pendingPartialMatchedModifierKeystrokes - bindingModifierKeyups = getModifierKeys(binding.keystrokeArray[binding.keystrokeArray.length-1]) - keystrokeModifierKeyups = getModifierKeys(keystroke) - if keystrokeModifierKeyups.length == 1 and bindingModifierKeyups.has(keystrokeModifierKeyups[0]) - exactMatchCandidates.push(binding) - # Ian TODO remove from @pendingPartialMatchedModifierKeystrokes - # Ian TODO deal with all the other partial match possibilities - # If any partial match *was* pending but has now failed to match, add it to # the list of bindings to disable so we don't attempt to match it again # during a subsequent event replay by `terminatePendingState`. @@ -546,6 +537,9 @@ class KeymapManager # immediately. Otherwise we break and allow ourselves to enter the pending # state with a timeout. if exactMatchCandidates.length > 0 + console.log('exactMatchCandidates:') + for c in exactMatchCandidates + console.log(c.keystrokes) currentTarget = target eventHandled = false while not eventHandled and currentTarget? and currentTarget isnt document @@ -573,6 +567,11 @@ class KeymapManager for partialMatch in partialMatches if pendingKeyupMatchCandidates.indexOf(partialMatch) < 0 allPartialMatchesContainKeyupRemainder = false + # We found one partial match with unmatched keydowns. + # We can stop looking. + break + # Don't dispatch this exact match. There are partial matches left + # that have keydowns. break if allPartialMatchesContainKeyupRemainder is false else shouldUsePartialMatches = false From c9b32cc7fc4acb988d290c236ea353d072490bcb Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Thu, 20 Oct 2016 17:13:12 -0700 Subject: [PATCH 09/15] keyup matcher class in progress --- spec/partial-keyup-matcher-spec.js | 21 +++++++++++++++++ src/key-binding.coffee | 9 +++++++- src/partial-keyup-matcher.js | 36 ++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 spec/partial-keyup-matcher-spec.js create mode 100644 src/partial-keyup-matcher.js diff --git a/spec/partial-keyup-matcher-spec.js b/spec/partial-keyup-matcher-spec.js new file mode 100644 index 0000000..43160c2 --- /dev/null +++ b/spec/partial-keyup-matcher-spec.js @@ -0,0 +1,21 @@ +'use babel' + +const PartialKeyupMatcher = require('../src/partial-keyup-matcher.js') +import {KeyBinding} from '../src/key-binding' + +describe("PartialKeyupMatcher", () => { + + let matcher = new PartialKeyupMatcher() + + it("blah", () => { + const kb = keyBindingArgHelper('ctrl-tab ^ctrl'); + matcher.addPendingMatch(kb) + const matches = matcher.getMatches('^ctrl') + assert.equal(matches.length, 1) + assert.equal(matches[0], kb) + }) +}) + +function keyBindingArgHelper(binding) { + return new KeyBinding('test', 'test', binding, 'body', 0) +} diff --git a/src/key-binding.coffee b/src/key-binding.coffee index ed14598..191127c 100644 --- a/src/key-binding.coffee +++ b/src/key-binding.coffee @@ -19,7 +19,7 @@ class KeyBinding @selector = selector.replace(/!important/g, '') @specificity = calculateSpecificity(selector) @index = @constructor.currentIndex++ - @isMatchedKeydownKeyupCache = null + @cachedKeyups = null matches: (keystroke) -> multiKeystroke = /\s/.test keystroke @@ -37,6 +37,13 @@ class KeyBinding else keyBinding.priority - @priority + # Return the keyup portion of the binding, if any, as an array of + # keystrokes. + getKeyups: -> + return @cachedKeyups if @cachedKeyups? + for keystroke, i in @keystrokeArray + return @cachedKeyups = @keystrokeArray.slice(i) if isKeyup(keystroke) + # userKeystrokes is an array of keystrokes e.g. # ['ctrl-y', 'ctrl-x', '^x'] matchesKeystrokes: (userKeystrokes) -> diff --git a/src/partial-keyup-matcher.js b/src/partial-keyup-matcher.js new file mode 100644 index 0000000..94b5418 --- /dev/null +++ b/src/partial-keyup-matcher.js @@ -0,0 +1,36 @@ +'use babel' + +module.exports = +class PartialKeyupMatcher { // Ian TODO this name sucks + + constructor() { + this._pendingMatches = new Set() + } + + addPendingMatch(keyBinding) { + this._pendingMatches.add(keyBinding) + keyBinding['nextKeyUpIndex'] = 0 + } + + // Returns matching bindings(s) if any. + // Updates state for next match. + getMatches(keyupKeystroke) { + let matches = [] + for (let keyBinding of this._pendingMatches) { + let toMatch = keyBinding.getKeyups()[keyBinding['nextKeyUpIndex']] + if (keyupKeystroke == toMatch) { + if (keyBinding['nextKeyUpIndex'] == keyBinding.getKeyups().length-1) { + // Full match. Remove and return it. + this._pendingMatches.delete(keyBinding) + matches.push(keyBinding) + } + else { + // Partial match. Increment what we're looking for next. + keyBinding['nextKeyUpIndex']++ + } + } + } + return matches + } + +} From b5d6d15b355cdd1e47aa527028eedce1a3689f09 Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Fri, 21 Oct 2016 15:04:22 -0700 Subject: [PATCH 10/15] lint fixes --- spec/helpers-spec.coffee | 24 ++++++++++++------------ spec/partial-keyup-matcher-spec.js | 13 +++++++------ src/partial-keyup-matcher.js | 13 ++++++------- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/spec/helpers-spec.coffee b/spec/helpers-spec.coffee index f5a1e70..1727bdb 100644 --- a/spec/helpers-spec.coffee +++ b/spec/helpers-spec.coffee @@ -34,17 +34,17 @@ describe ".normalizeKeystrokes(keystrokes)", -> describe ".isModifierKeyup(keystroke)", -> it "returns true for single modifier keyups", -> - assert(isModifierKeyup('^ctrl')) - assert(isModifierKeyup('^shift')) - assert(isModifierKeyup('^alt')) - assert(isModifierKeyup('^cmd')) - assert(isModifierKeyup('^ctrl-shift')) - assert(isModifierKeyup('^alt-cmd')) + assert.isTrue(isModifierKeyup('^ctrl')) + assert.isTrue(isModifierKeyup('^shift')) + assert.isTrue(isModifierKeyup('^alt')) + assert.isTrue(isModifierKeyup('^cmd')) + assert.isTrue(isModifierKeyup('^ctrl-shift')) + assert.isTrue(isModifierKeyup('^alt-cmd')) it "returns false for modifier keydowns", -> - assert(!isModifierKeyup('ctrl-x')) - assert(!isModifierKeyup('shift-x')) - assert(!isModifierKeyup('alt-x')) - assert(!isModifierKeyup('cmd-x')) - assert(!isModifierKeyup('ctrl-shift-x')) - assert(!isModifierKeyup('alt-cmd-x')) + assert.isFalse(isModifierKeyup('ctrl-x')) + assert.isFalse(isModifierKeyup('shift-x')) + assert.isFalse(isModifierKeyup('alt-x')) + assert.isFalse(isModifierKeyup('cmd-x')) + assert.isFalse(isModifierKeyup('ctrl-shift-x')) + assert.isFalse(isModifierKeyup('alt-cmd-x')) diff --git a/spec/partial-keyup-matcher-spec.js b/spec/partial-keyup-matcher-spec.js index 43160c2..a5e53ae 100644 --- a/spec/partial-keyup-matcher-spec.js +++ b/spec/partial-keyup-matcher-spec.js @@ -1,14 +1,15 @@ -'use babel' +/** @babel */ +/* eslint-env mocha */ +/* global assert */ const PartialKeyupMatcher = require('../src/partial-keyup-matcher.js') import {KeyBinding} from '../src/key-binding' -describe("PartialKeyupMatcher", () => { - +describe('PartialKeyupMatcher', () => { let matcher = new PartialKeyupMatcher() - it("blah", () => { - const kb = keyBindingArgHelper('ctrl-tab ^ctrl'); + it('returns a simple single-modifier-keyup match', () => { + const kb = keyBindingArgHelper('ctrl-tab ^ctrl') matcher.addPendingMatch(kb) const matches = matcher.getMatches('^ctrl') assert.equal(matches.length, 1) @@ -16,6 +17,6 @@ describe("PartialKeyupMatcher", () => { }) }) -function keyBindingArgHelper(binding) { +function keyBindingArgHelper (binding) { return new KeyBinding('test', 'test', binding, 'body', 0) } diff --git a/src/partial-keyup-matcher.js b/src/partial-keyup-matcher.js index 94b5418..e2e2164 100644 --- a/src/partial-keyup-matcher.js +++ b/src/partial-keyup-matcher.js @@ -3,28 +3,27 @@ module.exports = class PartialKeyupMatcher { // Ian TODO this name sucks - constructor() { + constructor () { this._pendingMatches = new Set() } - addPendingMatch(keyBinding) { + addPendingMatch (keyBinding) { this._pendingMatches.add(keyBinding) keyBinding['nextKeyUpIndex'] = 0 } // Returns matching bindings(s) if any. // Updates state for next match. - getMatches(keyupKeystroke) { + getMatches (keyupKeystroke) { let matches = [] for (let keyBinding of this._pendingMatches) { let toMatch = keyBinding.getKeyups()[keyBinding['nextKeyUpIndex']] - if (keyupKeystroke == toMatch) { - if (keyBinding['nextKeyUpIndex'] == keyBinding.getKeyups().length-1) { + if (keyupKeystroke === toMatch) { + if (keyBinding['nextKeyUpIndex'] === keyBinding.getKeyups().length - 1) { // Full match. Remove and return it. this._pendingMatches.delete(keyBinding) matches.push(keyBinding) - } - else { + } else { // Partial match. Increment what we're looking for next. keyBinding['nextKeyUpIndex']++ } From 800dc79dcbb46b57c25d3421627771b67a0f6107 Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Fri, 21 Oct 2016 18:00:13 -0700 Subject: [PATCH 11/15] PartialKeyupMatcher basically works, has tests --- spec/partial-keyup-matcher-spec.js | 60 +++++++++++++++++++++++++++++- src/partial-keyup-matcher.js | 57 ++++++++++++++++++++-------- 2 files changed, 100 insertions(+), 17 deletions(-) diff --git a/spec/partial-keyup-matcher-spec.js b/spec/partial-keyup-matcher-spec.js index a5e53ae..2565053 100644 --- a/spec/partial-keyup-matcher-spec.js +++ b/spec/partial-keyup-matcher-spec.js @@ -6,14 +6,70 @@ const PartialKeyupMatcher = require('../src/partial-keyup-matcher.js') import {KeyBinding} from '../src/key-binding' describe('PartialKeyupMatcher', () => { - let matcher = new PartialKeyupMatcher() - it('returns a simple single-modifier-keyup match', () => { + const matcher = new PartialKeyupMatcher() const kb = keyBindingArgHelper('ctrl-tab ^ctrl') matcher.addPendingMatch(kb) const matches = matcher.getMatches('^ctrl') assert.equal(matches.length, 1) assert.equal(matches[0], kb) + it('removes match returned', () => { + const matches = matcher.getMatches('^ctrl') + assert.equal(matches.length, 0) + }) + }) + + it('matches single keyups binding on multiple keyup event', () => { + const matcher = new PartialKeyupMatcher() + const kb = keyBindingArgHelper('ctrl-tab ^ctrl') + matcher.addPendingMatch(kb) + const matches = matcher.getMatches('^ctrl-shift') + assert.equal(matches.length, 1) + assert.equal(matches[0], kb) + it('removes match returned', () => { + const matches = matcher.getMatches('^ctrl') + assert.equal(matches.length, 0) + }) + }) + + it('does not match multiple keyup binding on single keyup events', () => { + const matcher = new PartialKeyupMatcher() + const kb = keyBindingArgHelper('ctrl-shift-tab ^ctrl-shift') + matcher.addPendingMatch(kb) + let matches = matcher.getMatches('^ctrl') + assert.equal(matches.length, 0) + matches = matcher.getMatches('^shift') + assert.equal(matches.length, 0) + }) + it('matches multiple keyup binding on multiple keyup events', () => { + const matcher = new PartialKeyupMatcher() + const kb = keyBindingArgHelper('ctrl-shift-tab ^ctrl-shift') + matcher.addPendingMatch(kb) + let matches = matcher.getMatches('^ctrl-shift') + assert.equal(matches.length, 1) + assert.equal(matches[0], kb) + it('removes match returned', () => { + const matches = matcher.getMatches('^ctrl') + assert.equal(matches.length, 0) + }) + }) + + it('for multi-keystroke bindings, matches only when all keyups are received', () => { + const matcher = new PartialKeyupMatcher() + const kb = keyBindingArgHelper('ctrl-shift-tab ^ctrl ^shift') + matcher.addPendingMatch(kb) + matches = matcher.getMatches('^shift') // no-op should return no match + assert.equal(matches.length, 0) + // should return no match but set state to match on next ^ctrl + let matches = matcher.getMatches('^ctrl') + assert.equal(matches.length, 0) + matches = matcher.getMatches('^shift') + assert.equal(matches.length, 1) + assert.equal(matches[0], kb) + it('removes match returned', () => { + const matches = matcher.getMatches('^ctrl') + assert.equal(matches.length, 0) + }) }) }) diff --git a/src/partial-keyup-matcher.js b/src/partial-keyup-matcher.js index e2e2164..3831bf4 100644 --- a/src/partial-keyup-matcher.js +++ b/src/partial-keyup-matcher.js @@ -1,7 +1,7 @@ 'use babel' module.exports = -class PartialKeyupMatcher { // Ian TODO this name sucks +class PartialKeyupMatcher { constructor () { this._pendingMatches = new Set() @@ -12,24 +12,51 @@ class PartialKeyupMatcher { // Ian TODO this name sucks keyBinding['nextKeyUpIndex'] = 0 } - // Returns matching bindings(s) if any. + // Returns matching bindingss, if any. // Updates state for next match. - getMatches (keyupKeystroke) { - let matches = [] - for (let keyBinding of this._pendingMatches) { - let toMatch = keyBinding.getKeyups()[keyBinding['nextKeyUpIndex']] - if (keyupKeystroke === toMatch) { - if (keyBinding['nextKeyUpIndex'] === keyBinding.getKeyups().length - 1) { - // Full match. Remove and return it. - this._pendingMatches.delete(keyBinding) - matches.push(keyBinding) - } else { - // Partial match. Increment what we're looking for next. - keyBinding['nextKeyUpIndex']++ + getMatches (userKeyupKeystroke) { + userKeyupKeystroke = this._normalizeKeystroke(userKeyupKeystroke) + let matches = new Set() + + // Loop over each pending keyup match. + for (const keyBinding of this._pendingMatches) { + const bindingKeystrokeToMatch = this._normalizeKeystroke(keyBinding.getKeyups()[keyBinding['nextKeyUpIndex']]) + const userKeyups = userKeyupKeystroke.split('-') + + // Attempt to match multi-keyup combinations e.g. ^ctrl-shift + if (userKeyups.length > 1) { + if (userKeyupKeystroke === bindingKeystrokeToMatch) { + this._updateStateForMatch(matches, keyBinding) + } + } + + // Loop over individual keys in the user keystroke because we want e.g. + // user keystroke ^ctrl-shift to match a pending ^ctrl or ^shift. + for (const userKeyup of userKeyups) { + if (userKeyup === bindingKeystrokeToMatch) { + this._updateStateForMatch(matches, keyBinding) } } } - return matches + return [...matches] + } + + /** Private Section **/ + + _normalizeKeystroke (keystroke) { + if (keystroke[0] === '^') return keystroke.substring(1) + return keystroke + } + + _updateStateForMatch (matches, keyBinding) { + if (keyBinding['nextKeyUpIndex'] === keyBinding.getKeyups().length - 1) { + // Full match. Remove and return it. + this._pendingMatches.delete(keyBinding) + matches.add(keyBinding) + } else { + // Partial match. Increment what we're looking for next. + keyBinding['nextKeyUpIndex']++ + } } } From 8fde502264dc4e3613fa071ef2c11191ec3e1b72 Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Fri, 21 Oct 2016 20:27:44 -0700 Subject: [PATCH 12/15] new behavior works well. old tests need fixing. --- spec/partial-keyup-matcher-spec.js | 25 ------------------------- src/helpers.coffee | 8 ++++---- src/keymap-manager.coffee | 12 +++++++----- src/partial-keyup-matcher.js | 21 +++++---------------- 4 files changed, 16 insertions(+), 50 deletions(-) diff --git a/spec/partial-keyup-matcher-spec.js b/spec/partial-keyup-matcher-spec.js index 2565053..fef2ef3 100644 --- a/spec/partial-keyup-matcher-spec.js +++ b/spec/partial-keyup-matcher-spec.js @@ -19,19 +19,6 @@ describe('PartialKeyupMatcher', () => { }) }) - it('matches single keyups binding on multiple keyup event', () => { - const matcher = new PartialKeyupMatcher() - const kb = keyBindingArgHelper('ctrl-tab ^ctrl') - matcher.addPendingMatch(kb) - const matches = matcher.getMatches('^ctrl-shift') - assert.equal(matches.length, 1) - assert.equal(matches[0], kb) - it('removes match returned', () => { - const matches = matcher.getMatches('^ctrl') - assert.equal(matches.length, 0) - }) - }) - it('does not match multiple keyup binding on single keyup events', () => { const matcher = new PartialKeyupMatcher() const kb = keyBindingArgHelper('ctrl-shift-tab ^ctrl-shift') @@ -41,18 +28,6 @@ describe('PartialKeyupMatcher', () => { matches = matcher.getMatches('^shift') assert.equal(matches.length, 0) }) - it('matches multiple keyup binding on multiple keyup events', () => { - const matcher = new PartialKeyupMatcher() - const kb = keyBindingArgHelper('ctrl-shift-tab ^ctrl-shift') - matcher.addPendingMatch(kb) - let matches = matcher.getMatches('^ctrl-shift') - assert.equal(matches.length, 1) - assert.equal(matches[0], kb) - it('removes match returned', () => { - const matches = matcher.getMatches('^ctrl') - assert.equal(matches.length, 0) - }) - }) it('for multi-keystroke bindings, matches only when all keyups are received', () => { const matcher = new PartialKeyupMatcher() diff --git a/src/helpers.coffee b/src/helpers.coffee index be33ab5..4666570 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -153,18 +153,18 @@ exports.keystrokeForKeyboardEvent = (event) -> key = characters.unmodified keystroke = '' - if key is 'ctrl' or ctrlKey + if key is 'ctrl' or (ctrlKey and event.type != 'keyup') keystroke += 'ctrl' - if key is 'alt' or altKey + if key is 'alt' or (altKey and event.type != 'keyup') keystroke += '-' if keystroke.length > 0 keystroke += 'alt' - if key is 'shift' or (shiftKey and (isNonCharacterKey or (isLatinCharacter(key) and isUpperCaseCharacter(key)))) + if key is 'shift' or (shiftKey and event.type != 'keyup' and (isNonCharacterKey or (isLatinCharacter(key) and isUpperCaseCharacter(key)))) keystroke += '-' if keystroke keystroke += 'shift' - if key is 'cmd' or metaKey + if key is 'cmd' or (metaKey and event.type != 'keyup') keystroke += '-' if keystroke keystroke += 'cmd' diff --git a/src/keymap-manager.coffee b/src/keymap-manager.coffee index f80f0cb..dc37e21 100644 --- a/src/keymap-manager.coffee +++ b/src/keymap-manager.coffee @@ -8,6 +8,7 @@ path = require 'path' {KeyBinding, MATCH_TYPES} = require './key-binding' CommandEvent = require './command-event' {normalizeKeystrokes, keystrokeForKeyboardEvent, isBareModifier, keydownEvent, keyupEvent, characterForKeyboardEvent, keystrokesMatch, isKeyup} = require './helpers' +PartialKeyupMatcher = require './partial-keyup-matcher' Platforms = ['darwin', 'freebsd', 'linux', 'sunos', 'win32'] OtherPlatforms = Platforms.filter (platform) -> platform isnt process.platform @@ -97,7 +98,7 @@ class KeymapManager # Pending matches to bindings that begin with keydowns and end with a subset # of matching keyups - pendingKeyupMatches: null + pendingKeyupMatcher: new PartialKeyupMatcher() ### Section: Construction and Destruction @@ -531,15 +532,15 @@ class KeymapManager hasPartialMatches = partialMatches.length > 0 shouldUsePartialMatches = hasPartialMatches + if isKeyup(keystroke) + exactMatchCandidates = exactMatchCandidates.concat(@pendingKeyupMatcher.getMatches(keystroke)) + # Determine if the current keystrokes match any bindings *exactly*. If we # do find an exact match, the next step depends on whether we have any # partial matches. If we have no partial matches, we dispatch the command # immediately. Otherwise we break and allow ourselves to enter the pending # state with a timeout. if exactMatchCandidates.length > 0 - console.log('exactMatchCandidates:') - for c in exactMatchCandidates - console.log(c.keystrokes) currentTarget = target eventHandled = false while not eventHandled and currentTarget? and currentTarget isnt document @@ -581,6 +582,8 @@ class KeymapManager console.log('dispatched: ' + exactMatchCandidate.keystrokes) dispatchedExactMatch = exactMatchCandidate eventHandled = true + for pendingKeyupMatch in pendingKeyupMatchCandidates + @pendingKeyupMatcher.addPendingMatch(pendingKeyupMatch) break currentTarget = currentTarget.parentElement @@ -677,7 +680,6 @@ class KeymapManager else if doesMatch is MATCH_TYPES.PARTIAL partialMatchCandidates.push(binding) else if doesMatch is MATCH_TYPES.PENDING_KEYUP - partialMatchCandidates.push(binding) pendingKeyupMatchCandidates.push(binding) {partialMatchCandidates, pendingKeyupMatchCandidates, exactMatchCandidates} diff --git a/src/partial-keyup-matcher.js b/src/partial-keyup-matcher.js index 3831bf4..08502fd 100644 --- a/src/partial-keyup-matcher.js +++ b/src/partial-keyup-matcher.js @@ -20,22 +20,11 @@ class PartialKeyupMatcher { // Loop over each pending keyup match. for (const keyBinding of this._pendingMatches) { - const bindingKeystrokeToMatch = this._normalizeKeystroke(keyBinding.getKeyups()[keyBinding['nextKeyUpIndex']]) - const userKeyups = userKeyupKeystroke.split('-') - - // Attempt to match multi-keyup combinations e.g. ^ctrl-shift - if (userKeyups.length > 1) { - if (userKeyupKeystroke === bindingKeystrokeToMatch) { - this._updateStateForMatch(matches, keyBinding) - } - } - - // Loop over individual keys in the user keystroke because we want e.g. - // user keystroke ^ctrl-shift to match a pending ^ctrl or ^shift. - for (const userKeyup of userKeyups) { - if (userKeyup === bindingKeystrokeToMatch) { - this._updateStateForMatch(matches, keyBinding) - } + const bindingKeystrokeToMatch = this._normalizeKeystroke( + keyBinding.getKeyups()[keyBinding['nextKeyUpIndex']] + ) + if (userKeyupKeystroke === bindingKeystrokeToMatch) { + this._updateStateForMatch(matches, keyBinding) } } return [...matches] From 1bd09a59b47b055b0e634ebb625a6bd338499e64 Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Mon, 24 Oct 2016 16:44:56 -0700 Subject: [PATCH 13/15] existing tests passing --- spec/keymap-manager-spec.coffee | 18 +++++++++++++++--- src/keymap-manager.coffee | 1 + 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/spec/keymap-manager-spec.coffee b/spec/keymap-manager-spec.coffee index c3e0b42..2a758dd 100644 --- a/spec/keymap-manager-spec.coffee +++ b/spec/keymap-manager-spec.coffee @@ -398,6 +398,7 @@ describe "KeymapManager", -> elementA.addEventListener 'x-command-ctrl-up', (e) -> events.push('x-ctrl-keyup') elementA.addEventListener 'y-command-y-up-ctrl-up', (e) -> events.push('y-up-ctrl-keyup') elementA.addEventListener 'abc-secret-code-command', (e) -> events.push('abc-secret-code') + elementA.addEventListener 'z-command-d-e-f', (e) -> events.push('z-keydown-d-e-f') keymapManager.add "test", ".a": @@ -406,6 +407,7 @@ describe "KeymapManager", -> "ctrl-x ^ctrl": "x-command-ctrl-up" "ctrl-y ^y ^ctrl": "y-command-y-up-ctrl-up" "a b c ^b ^a ^c": "abc-secret-code-command" + "ctrl-z d e f": "z-command-d-e-f" it "dispatches the command for a binding containing only keydown events immediately even when there is a corresponding multi-stroke binding that contains only other keyup events", -> keymapManager.handleKeyboardEvent(buildKeydownEvent('y', ctrl: true, target: elementA)) @@ -445,14 +447,24 @@ describe "KeymapManager", -> getFakeClock().tick(keymapManager.getPartialMatchTimeout()) assert.deepEqual(events, ['x-ctrl-keyup']) - it "does _not_ dispatch the command when extra user-generated keydown events are not specified in the binding", -> + it "dispatches the command when extra user-generated keydown events not specified in the binding occur between keydown and keyup", -> keymapManager.handleKeyboardEvent(buildKeydownEvent('y', ctrl: true, target: elementA)) assert.deepEqual(events, ['y-keydown']) - keymapManager.handleKeyboardEvent(buildKeydownEvent('z', ctrl: true, target: elementA)) # not specified in binding + keymapManager.handleKeyboardEvent(buildKeydownEvent('j', ctrl: true, target: elementA)) # not specified in binding assert.deepEqual(events, ['y-keydown']) keymapManager.handleKeyboardEvent(buildKeyupEvent('ctrl', target: elementA)) getFakeClock().tick(keymapManager.getPartialMatchTimeout()) - assert.deepEqual(events, ['y-keydown']) + assert.deepEqual(events, ['y-keydown', 'y-ctrl-keyup']) + + it "does _not_ dispatch the command when extra user-generated keydown events not specified in the binding occur between keydowns", -> + keymapManager.handleKeyboardEvent(buildKeydownEvent('z', ctrl: true, target: elementA)) + keymapManager.handleKeyboardEvent(buildKeyupEvent('ctrl', target: elementA)) + keymapManager.handleKeyboardEvent(buildKeyupEvent('ctrl', target: elementA)) + keymapManager.handleKeyboardEvent(buildKeydownEvent('z', target: elementA)) # not specified in binding + keymapManager.handleKeyboardEvent(buildKeydownEvent('d', target: elementA)) + keymapManager.handleKeyboardEvent(buildKeydownEvent('e', target: elementA)) + keymapManager.handleKeyboardEvent(buildKeydownEvent('f', target: elementA)) + assert.deepEqual(events, []) it "dispatches the command when multiple keyup keystrokes are specified", -> keymapManager.handleKeyboardEvent(buildKeydownEvent('a', target: elementA)) diff --git a/src/keymap-manager.coffee b/src/keymap-manager.coffee index dc37e21..8004c65 100644 --- a/src/keymap-manager.coffee +++ b/src/keymap-manager.coffee @@ -680,6 +680,7 @@ class KeymapManager else if doesMatch is MATCH_TYPES.PARTIAL partialMatchCandidates.push(binding) else if doesMatch is MATCH_TYPES.PENDING_KEYUP + partialMatchCandidates.push(binding) pendingKeyupMatchCandidates.push(binding) {partialMatchCandidates, pendingKeyupMatchCandidates, exactMatchCandidates} From d77ccbdbbc13d2d82894ea585c8cc0ebabb999aa Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Mon, 24 Oct 2016 16:56:01 -0700 Subject: [PATCH 14/15] one more test. cleanup. --- spec/keymap-manager-spec.coffee | 9 +++++++++ src/keymap-manager.coffee | 4 ---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/spec/keymap-manager-spec.coffee b/spec/keymap-manager-spec.coffee index 2a758dd..42562e1 100644 --- a/spec/keymap-manager-spec.coffee +++ b/spec/keymap-manager-spec.coffee @@ -422,6 +422,15 @@ describe "KeymapManager", -> getFakeClock().tick(keymapManager.getPartialMatchTimeout()) assert.deepEqual(events, ['y-keydown', 'y-up-ctrl-keyup']) + it "dispatches the command when they keyup comes after the partial match timeout", -> + keymapManager.handleKeyboardEvent(buildKeydownEvent('y', ctrl: true, target: elementA)) + assert.deepEqual(events, ['y-keydown']) + keymapManager.handleKeyboardEvent(buildKeyupEvent('y', ctrl: true, cmd: true, shift: true, alt: true, target: elementA)) + assert.deepEqual(events, ['y-keydown']) + getFakeClock().tick(keymapManager.getPartialMatchTimeout()) + keymapManager.handleKeyboardEvent(buildKeyupEvent('ctrl', target: elementA)) + assert.deepEqual(events, ['y-keydown', 'y-up-ctrl-keyup']) + it "dispatches the command multiple times when multiple keydown events for the binding come in before the binding with a keyup handler", -> keymapManager.handleKeyboardEvent(buildKeydownEvent('y', ctrl: true, target: elementA)) assert.deepEqual(events, ['y-keydown']) diff --git a/src/keymap-manager.coffee b/src/keymap-manager.coffee index 8004c65..b01cf83 100644 --- a/src/keymap-manager.coffee +++ b/src/keymap-manager.coffee @@ -497,8 +497,6 @@ class KeymapManager # keystroke is the atom keybind syntax, e.g. 'ctrl-a' keystroke = @keystrokeForKeyboardEvent(event) - # Ian TODO :fire: - console.log(keystroke) # We dont care about bare modifier keys in the bindings. e.g. `ctrl y` isnt going to work. if event.type is 'keydown' and @queuedKeystrokes.length > 0 and isBareModifier(keystroke) @@ -578,8 +576,6 @@ class KeymapManager shouldUsePartialMatches = false if @dispatchCommandEvent(exactMatchCandidate.command, target, event) - # Ian TODO :fire: - console.log('dispatched: ' + exactMatchCandidate.keystrokes) dispatchedExactMatch = exactMatchCandidate eventHandled = true for pendingKeyupMatch in pendingKeyupMatchCandidates From 4fc8165b41f89ca4e9aaa22b4b47ec680e7aa062 Mon Sep 17 00:00:00 2001 From: Ian Olsen Date: Mon, 24 Oct 2016 19:32:26 -0700 Subject: [PATCH 15/15] lint fixes --- spec/keymap-manager-spec.coffee | 8 ++++---- src/helpers.coffee | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/keymap-manager-spec.coffee b/spec/keymap-manager-spec.coffee index 84e2e73..8e0dca9 100644 --- a/spec/keymap-manager-spec.coffee +++ b/spec/keymap-manager-spec.coffee @@ -420,13 +420,13 @@ describe "KeymapManager", -> getFakeClock().tick(keymapManager.getPartialMatchTimeout()) assert.deepEqual(events, ['y-keydown', 'y-up-ctrl-keyup']) - it "dispatches the command when they keyup comes after the partial match timeout", -> - keymapManager.handleKeyboardEvent(buildKeydownEvent('y', ctrl: true, target: elementA)) + it "dispatches the command when the keyup comes after the partial match timeout", -> + keymapManager.handleKeyboardEvent(buildKeydownEvent(key: 'y', ctrlKey: true, target: elementA)) assert.deepEqual(events, ['y-keydown']) - keymapManager.handleKeyboardEvent(buildKeyupEvent('y', ctrl: true, cmd: true, shift: true, alt: true, target: elementA)) + keymapManager.handleKeyboardEvent(buildKeyupEvent(key: 'y', ctrlKey: true, cmd: true, shift: true, alt: true, target: elementA)) assert.deepEqual(events, ['y-keydown']) getFakeClock().tick(keymapManager.getPartialMatchTimeout()) - keymapManager.handleKeyboardEvent(buildKeyupEvent('ctrl', target: elementA)) + keymapManager.handleKeyboardEvent(buildKeyupEvent(key: 'ctrl', target: elementA)) assert.deepEqual(events, ['y-keydown', 'y-up-ctrl-keyup']) it "dispatches the command multiple times when multiple keydown events for the binding come in before the binding with a keyup handler", -> diff --git a/src/helpers.coffee b/src/helpers.coffee index bd1d1ff..fb56d78 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -182,18 +182,18 @@ exports.keystrokeForKeyboardEvent = (event) -> key = characters.unmodified keystroke = '' - if key is 'ctrl' or (ctrlKey and event.type != 'keyup') + if key is 'ctrl' or (ctrlKey and event.type isnt 'keyup') keystroke += 'ctrl' - if key is 'alt' or (altKey and event.type != 'keyup') + if key is 'alt' or (altKey and event.type isnt 'keyup') keystroke += '-' if keystroke.length > 0 keystroke += 'alt' - if key is 'shift' or (shiftKey and event.type != 'keyup' and (isNonCharacterKey or (isLatinCharacter(key) and isUpperCaseCharacter(key)))) + if key is 'shift' or (shiftKey and event.type isnt 'keyup' and (isNonCharacterKey or (isLatinCharacter(key) and isUpperCaseCharacter(key)))) keystroke += '-' if keystroke keystroke += 'shift' - if key is 'cmd' or (metaKey and event.type != 'keyup') + if key is 'cmd' or (metaKey and event.type isnt 'keyup') keystroke += '-' if keystroke keystroke += 'cmd'