Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vim use of keydown and keypress preventing codemirrorIgnore? #2915

Closed
dalejung opened this issue Nov 7, 2014 · 14 comments
Closed

vim use of keydown and keypress preventing codemirrorIgnore? #2915

dalejung opened this issue Nov 7, 2014 · 14 comments
Labels

Comments

@dalejung
Copy link
Contributor

dalejung commented Nov 7, 2014

As far as I can tell, the move to on('keydown') and on('keypress')` is circumventing the normal way of preventing codemirror from doing things with events.

What's the proper way to register a keyevent handler that can prevent the keymap from firing?

@dalejung
Copy link
Contributor Author

dalejung commented Nov 7, 2014

diff --git i/keymap/vim.js w/keymap/vim.js
index 558e4c8..f884f7b 100644
--- i/keymap/vim.js
+++ w/keymap/vim.js
@@ -251,6 +251,7 @@
     // Keys with modifiers are handled using keydown due to limitations of
     // keypress event.
     function handleKeyDown(cm, e) {
+      if(e.codemirrorIgnore) { return; }
       var name = lookupKey(e);
       if (!name) { return; }

@@ -262,6 +263,7 @@
     // Keys without modifiers are handled using keypress to work best with
     // non-standard keyboard layouts.
     function handleKeyPress(cm, e) {
+      if(e.codemirrorIgnore) { return; }
       var code = e.charCode || e.keyCode;
       if (e.ctrlKey || e.metaKey || e.altKey ||
           e.shiftKey && code < 32) { return; }

this helped give me the behavior that I wanted. The other issue is that I have to modify the _handlers array directly since on() doesn't give me the ability to insert a handler before the vim keymap handlers.

@mightyguava mightyguava added the vim label Nov 9, 2014
@mightyguava
Copy link
Contributor

You could register your handler before turning on the vim mode, i.e.

var editor = CodeMirror.fromTextArea(...);
// register handlers
editor.setOption('vimMode', true);

and that should put your handlers before vim gets it.

@dalejung
Copy link
Contributor Author

dalejung commented Nov 9, 2014

How would I cancel vim handling for certain events?

@mightyguava
Copy link
Contributor

I suppose vim mode could check for preventDefault and codemirrorIgnore values on the event and ignore it if either is set. Would that solve your problem?

@dalejung
Copy link
Contributor Author

dalejung commented Nov 9, 2014

Yeah, that how I patched my local CodeMirror. I think it's the wrong place to handle it, but we'd need to split handlers into app and internal in order to remove it.

@mightyguava
Copy link
Contributor

Tricky problem. @marijnh do you have any suggestions on a good way for clients to suppress keypress and keydown handlers registered on CodeMirror instances?

@marijnh
Copy link
Member

marijnh commented Nov 11, 2014

I think it would be preferable to move the vim mode's key handling down into the keymap layer again, which has at least some concept of priorities and fine-grained handling. Would it work for you to be able to add a function to a keymap that is simply called for each key, and which can return a value indicating whether it chose to handle that key? You'd have to translate from CodeMirror-style key names to Vim-style names, but that's probably not very hard.

@mightyguava
Copy link
Contributor

Yes, that's probably a good idea. I didn't realize until after I finished the insert mode key handling work that I didn't actually have to move the key handling out of the keymap layer.

A function on the keymap that is called for each key would make the vim code cleaner if we take this approach.

@marijnh
Copy link
Member

marijnh commented Nov 12, 2014

Attached patch should allow you to give the vim keymap(s) a call method (or just make them functions). This method will be called with the name of a key, and should return null if there is no binding, and a function when there is one. That function will then be called with a CodeMirror instance as argument to actually execute the binding's action.

You might also want to take a look at the new functionality for multi-stroke key bindings. Basically, if you return "..." from a keymap, the key will be stored, and combined with the next key, so that if you press Ctrl-X and that returns "...", and then you press F, the string "Ctrl-X F" will be dispatched to the keymap. This might be too primitive for the vim map's use, I'm not sure.

@marijnh
Copy link
Member

marijnh commented Nov 12, 2014

By the way, if you need access to the editor state during dispatch, I am also open to passing the editor instance as second argument to the keymap function. It's less clean, but I could live with it.

@mightyguava
Copy link
Contributor

I took a crack at this today, and realized I do need the editor instance. Vim mode's key mappings can change depending on its current state

@marijnh
Copy link
Member

marijnh commented Nov 22, 2014

Attached patch causes the editor instance to be passed as second argument to the keymap's call method.

@cldwalker
Copy link
Contributor

I believe this issue is preventing Light Table's Vim from upgrading - see LightTable/Vim#45

@mightyguava
Copy link
Contributor

@cldwalker This should be resolved on Monday when I sync up with @heppe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants