From a2cf77892d777e57e1af7efbde246f6f6ea5bda7 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 12 Jan 2012 17:51:23 -0800 Subject: [PATCH] Don't insert characters in vim command mode GlobalKeymap.bindKey can take a selector and a custom function that maps key events to commands. If it returns false, no command is triggered but event propagation is halted. --- spec/atom/global-keymap-spec.coffee | 67 +++++++-- spec/atom/global-keymap-spec.js | 217 ++++++++++++++++++++++++++++ spec/atom/vim-mode-spec.coffee | 8 +- src/atom/binding-set.coffee | 13 +- src/atom/global-keymap.coffee | 5 +- src/atom/vim-mode.coffee | 1 + 6 files changed, 296 insertions(+), 15 deletions(-) create mode 100644 spec/atom/global-keymap-spec.js diff --git a/spec/atom/global-keymap-spec.coffee b/spec/atom/global-keymap-spec.coffee index 07926a765..286a5d503 100644 --- a/spec/atom/global-keymap-spec.coffee +++ b/spec/atom/global-keymap-spec.coffee @@ -2,13 +2,20 @@ GlobalKeymap = require 'global-keymap' $ = require 'jquery' describe "GlobalKeymap", -> + fragment = null keymap = null beforeEach -> keymap = new GlobalKeymap + fragment = $ """ +
+
+
+
+
+ """ - describe "handleKeyEvent", -> - fragment = null + describe ".handleKeyEvent(event)", -> deleteCharHandler = null insertCharHandler = null @@ -16,14 +23,6 @@ describe "GlobalKeymap", -> keymap.bindKeys '.command-mode', 'x': 'deleteChar' keymap.bindKeys '.insert-mode', 'x': 'insertChar' - fragment = $ """ -
-
-
-
-
- """ - deleteCharHandler = jasmine.createSpy 'deleteCharHandler' insertCharHandler = jasmine.createSpy 'insertCharHandler' fragment.on 'deleteChar', deleteCharHandler @@ -120,5 +119,53 @@ describe "GlobalKeymap", -> keymap.handleKeyEvent(keypressEvent('y', target: target)) expect(bazHandler).toHaveBeenCalled() + describe ".bindAllKeys(fn)", -> + it "calls given fn when selector matches", -> + handler = jasmine.createSpy 'handler' + keymap.bindKeys '.child-node', handler + target = fragment.find('.grandchild-node')[0] + event = keypressEvent('y', target: target) + keymap.handleKeyEvent event + + expect(handler).toHaveBeenCalledWith(event) + + describe "when the handler function returns a command string", -> + it "triggers the command event on the target and stops propagating the event", -> + keymap.bindKeys '*', 'x': 'foo' + keymap.bindKeys '*', -> 'bar' + fooHandler = jasmine.createSpy('fooHandler') + barHandler = jasmine.createSpy('barHandler') + fragment.on 'foo', fooHandler + fragment.on 'bar', barHandler + + target = fragment.find('.child-node')[0] + keymap.handleKeyEvent(keydownEvent('x', target: target)) + + expect(fooHandler).not.toHaveBeenCalled() + expect(barHandler).toHaveBeenCalled() + + describe "when the handler function returns false", -> + it "stops propagating the event", -> + keymap.bindKeys '*', 'x': 'foo' + keymap.bindKeys '*', -> false + fooHandler = jasmine.createSpy('fooHandler') + fragment.on 'foo', fooHandler + + target = fragment.find('.child-node')[0] + keymap.handleKeyEvent(keydownEvent('x', target: target)) + + expect(fooHandler).not.toHaveBeenCalled() + + describe "when the handler function returns anything other than a string or false", -> + it "continues to propagate the event", -> + keymap.bindKeys '*', 'x': 'foo' + keymap.bindKeys '*', -> undefined + fooHandler = jasmine.createSpy('fooHandler') + fragment.on 'foo', fooHandler + + target = fragment.find('.child-node')[0] + keymap.handleKeyEvent(keydownEvent('x', target: target)) + + expect(fooHandler).toHaveBeenCalled() diff --git a/spec/atom/global-keymap-spec.js b/spec/atom/global-keymap-spec.js new file mode 100644 index 000000000..58ba847d7 --- /dev/null +++ b/spec/atom/global-keymap-spec.js @@ -0,0 +1,217 @@ +(function() { + var $, GlobalKeymap; + GlobalKeymap = require('global-keymap'); + $ = require('jquery'); + describe("GlobalKeymap", function() { + var fragment, keymap; + fragment = null; + keymap = null; + beforeEach(function() { + keymap = new GlobalKeymap; + return fragment = $("
\n
\n
\n
\n
"); + }); + describe(".handleKeyEvent(event)", function() { + var deleteCharHandler, insertCharHandler; + deleteCharHandler = null; + insertCharHandler = null; + beforeEach(function() { + keymap.bindKeys('.command-mode', { + 'x': 'deleteChar' + }); + keymap.bindKeys('.insert-mode', { + 'x': 'insertChar' + }); + deleteCharHandler = jasmine.createSpy('deleteCharHandler'); + insertCharHandler = jasmine.createSpy('insertCharHandler'); + fragment.on('deleteChar', deleteCharHandler); + return fragment.on('insertChar', insertCharHandler); + }); + describe("when no binding matches the event", function() { + return it("returns true, so the event continues to propagate", function() { + return expect(keymap.handleKeyEvent(keypressEvent('0', { + target: fragment[0] + }))).toBeTruthy(); + }); + }); + describe("when the event's target node matches a selector with a matching binding", function() { + return it("triggers the command event associated with that binding on the target node and returns false", function() { + var commandEvent, event, result; + result = keymap.handleKeyEvent(keypressEvent('x', { + target: fragment[0] + })); + expect(result).toBe(false); + expect(deleteCharHandler).toHaveBeenCalled(); + expect(insertCharHandler).not.toHaveBeenCalled(); + deleteCharHandler.reset(); + fragment.removeClass('command-mode').addClass('insert-mode'); + event = keypressEvent('x', { + target: fragment[0] + }); + keymap.handleKeyEvent(event); + expect(deleteCharHandler).not.toHaveBeenCalled(); + expect(insertCharHandler).toHaveBeenCalled(); + commandEvent = insertCharHandler.argsForCall[0][0]; + expect(commandEvent.keyEvent).toBe(event); + return expect(event.char).toBe('x'); + }); + }); + describe("when the event's target node *descends* from a selector with a matching binding", function() { + return it("triggers the command event associated with that binding on the target node and returns false", function() { + var result, target; + target = fragment.find('.child-node')[0]; + result = keymap.handleKeyEvent(keypressEvent('x', { + target: target + })); + expect(result).toBe(false); + expect(deleteCharHandler).toHaveBeenCalled(); + expect(insertCharHandler).not.toHaveBeenCalled(); + deleteCharHandler.reset(); + fragment.removeClass('command-mode').addClass('insert-mode'); + keymap.handleKeyEvent(keypressEvent('x', { + target: target + })); + expect(deleteCharHandler).not.toHaveBeenCalled(); + return expect(insertCharHandler).toHaveBeenCalled(); + }); + }); + describe("when the event's target node descends from multiple nodes that match selectors with a binding", function() { + return it("only triggers bindings on selectors associated with the closest ancestor node", function() { + var fooHandler, target; + keymap.bindKeys('.child-node', { + 'x': 'foo' + }); + fooHandler = jasmine.createSpy('fooHandler'); + fragment.on('foo', fooHandler); + target = fragment.find('.grandchild-node')[0]; + keymap.handleKeyEvent(keypressEvent('x', { + target: target + })); + expect(fooHandler).toHaveBeenCalled(); + expect(deleteCharHandler).not.toHaveBeenCalled(); + return expect(insertCharHandler).not.toHaveBeenCalled(); + }); + }); + return describe("when the event bubbles to a node that matches multiple selectors", function() { + describe("when the matching selectors differ in specificity", function() { + return it("triggers the binding for the most specific selector", function() { + var barHandler, bazHandler, fooHandler, target; + keymap.bindKeys('div .child-node', { + 'x': 'foo' + }); + keymap.bindKeys('.command-mode .child-node', { + 'x': 'baz' + }); + keymap.bindKeys('.child-node', { + 'x': 'bar' + }); + fooHandler = jasmine.createSpy('fooHandler'); + barHandler = jasmine.createSpy('barHandler'); + bazHandler = jasmine.createSpy('bazHandler'); + fragment.on('foo', fooHandler); + fragment.on('bar', barHandler); + fragment.on('baz', bazHandler); + target = fragment.find('.grandchild-node')[0]; + keymap.handleKeyEvent(keypressEvent('x', { + target: target + })); + expect(fooHandler).not.toHaveBeenCalled(); + expect(barHandler).not.toHaveBeenCalled(); + return expect(bazHandler).toHaveBeenCalled(); + }); + }); + return describe("when the matching selectors have the same specificity", function() { + return it("triggers the bindings for the most recently declared selector", function() { + var barHandler, bazHandler, fooHandler, target; + keymap.bindKeys('.child-node', { + 'x': 'foo', + 'y': 'baz' + }); + keymap.bindKeys('.child-node', { + 'x': 'bar' + }); + fooHandler = jasmine.createSpy('fooHandler'); + barHandler = jasmine.createSpy('barHandler'); + bazHandler = jasmine.createSpy('bazHandler'); + fragment.on('foo', fooHandler); + fragment.on('bar', barHandler); + fragment.on('baz', bazHandler); + target = fragment.find('.grandchild-node')[0]; + keymap.handleKeyEvent(keypressEvent('x', { + target: target + })); + expect(barHandler).toHaveBeenCalled(); + expect(fooHandler).not.toHaveBeenCalled(); + keymap.handleKeyEvent(keypressEvent('y', { + target: target + })); + return expect(bazHandler).toHaveBeenCalled(); + }); + }); + }); + }); + return fdescribe(".bindAllKeys(fn)", function() { + it("calls given fn when selector matches", function() { + var event, handler, target; + handler = jasmine.createSpy('handler'); + keymap.bindAllKeys('.child-node', handler); + target = fragment.find('.grandchild-node')[0]; + event = keypressEvent('y', { + target: target + }); + keymap.handleKeyEvent(event); + return expect(handler).toHaveBeenCalledWith(event); + }); + describe("when the handler function returns a command string", function() { + return it("triggers the command event on the target and stops propagating the event", function() { + var barHandler, fooHandler; + keymap.bindKeys('*', { + 'x': 'foo' + }); + keymap.bindAllKeys('*', function() { + return 'bar'; + }); + fooHandler = jasmine.createSpy('fooHandler'); + barHandler = jasmine.createSpy('barHandler'); + fragment.on('foo', fooHandler); + fragment.on('bar', barHandler); + keymap.handleKeyEvent(keydownEvent('x')); + expect(fooHandler).not.toHaveBeenCalled(); + return expect(barHandler).toHaveBeenCalled(); + }); + }); + describe("when the handler function returns false", function() { + return it("stops propagating the event", function() { + var fooHandler; + keymap.bindKeys('*', { + 'x': 'foo' + }); + keymap.bindAllKeys('*', function() { + return false; + }); + fooHandler = jasmine.createSpy('fooHandler'); + fragment.on('foo', fooHandler); + keymap.handleKeyEvent(keydownEvent('x')); + return expect(fooHandler).not.toHaveBeenCalled(); + }); + }); + return describe("when the handler function returns anything other than a string or false", function() { + return it("continues to propagate the event", function() { + var fooHandler, target; + keymap.bindKeys('*', { + 'x': 'foo' + }); + keymap.bindAllKeys('*', function() { + return; + }); + fooHandler = jasmine.createSpy('fooHandler'); + fragment.on('foo', fooHandler); + target = fragment.find('.child-node')[0]; + keymap.handleKeyEvent(keydownEvent('x', { + target: target + })); + return expect(fooHandler).toHaveBeenCalled(); + }); + }); + }); + }); +}).call(this); diff --git a/spec/atom/vim-mode-spec.coffee b/spec/atom/vim-mode-spec.coffee index f4d6b8ebf..22da7e256 100644 --- a/spec/atom/vim-mode-spec.coffee +++ b/spec/atom/vim-mode-spec.coffee @@ -13,7 +13,13 @@ describe "VimMode", -> it "puts the editor in command-mode initially", -> expect(editor).toHaveClass 'command-mode' - describe "command-mode", -> + fdescribe "command-mode", -> + it "stops propagation on key events that don't have bindings so that they don't get inserted", -> + event = keydownEvent('\\') + spyOn(event, 'stopPropagation') + editor.trigger event + expect(event.stopPropagation).toHaveBeenCalled() + describe "the i keybinding", -> it "puts the editor into insert mode", -> expect(editor).not.toHaveClass 'insert-mode' diff --git a/src/atom/binding-set.coffee b/src/atom/binding-set.coffee index c248a97c4..f4afbd51c 100644 --- a/src/atom/binding-set.coffee +++ b/src/atom/binding-set.coffee @@ -1,3 +1,4 @@ +_ = require 'underscore' $ = require 'jquery' Specificity = require 'specificity' @@ -11,13 +12,19 @@ class BindingSet '=': 187, ';': 186, '\'': 222, '[': 219, ']': 221, '\\': 220 selector: null - bindings: null + bindingMap: null + bindingFunction: null - constructor: (@selector, @bindings) -> + constructor: (@selector, mapOrFunction) -> + if _.isFunction(mapOrFunction) + @bindingFunction = mapOrFunction + else + @bindingMap = mapOrFunction @specificity = Specificity(@selector) commandForEvent: (event) -> - for pattern, command of @bindings + return @bindingFunction(event) if @bindingFunction + for pattern, command of @bindingMap return command if @eventMatchesPattern(event, pattern) null diff --git a/src/atom/global-keymap.coffee b/src/atom/global-keymap.coffee index a51c35faf..4dfa5d68b 100644 --- a/src/atom/global-keymap.coffee +++ b/src/atom/global-keymap.coffee @@ -18,9 +18,12 @@ class GlobalKeymap candidateBindingSets = @bindingSets.filter (set) -> currentNode.is(set.selector) candidateBindingSets.sort (a, b) -> b.specificity - a.specificity for bindingSet in candidateBindingSets - if command = bindingSet.commandForEvent(event) + command = bindingSet.commandForEvent(event) + if command @triggerCommandEvent(event, command) return false + else if command == false + return false currentNode = currentNode.parent() true diff --git a/src/atom/vim-mode.coffee b/src/atom/vim-mode.coffee index e7db89ebe..c2fd11b71 100644 --- a/src/atom/vim-mode.coffee +++ b/src/atom/vim-mode.coffee @@ -9,6 +9,7 @@ class VimMode constructor: (@editor) -> @opStack = [] + atom.bindKeys '.command-mode', > false atom.bindKeys '.command-mode', @commandModeBindings() atom.bindKeys '.insert-mode', '': 'command-mode:activate'