SPC g causes atom-keymap to start failing

[jackcasey]

On a pretty vanilla install of atom and proton-mode I am having trouble with the SPC menu. But only with SPC g. SPC f s etc are fine.

After using SPC g I get errors about atom-keymap/lib/partial-keyup-matcher.js.

I'm eliminated everything I can thing of WRT extra packages or my own customisations around keymaps etc etc.

Anyone else having this recently?

[jackcasey]

Update: This also happens when you push the g in SPC t g. Which is interesting... So it's not an issue with the git menu but something to do with the 'g' itself.

[geksilla]

Observes same issue. Don't know why but it starts to throw errors when vim-mode-plus enabled. If you open Key Binding resolver and hit SPC g you'll see a lot of key bindings matched to g which waits for next key. Have no idea if this an issue of vim-mode-plus or atom itself.

[geksilla]

@jackcasey I just tried to check different versions of vim-mode-plus and found that this issue happens since 0.99.0. So as workaround try downgrade vim-mode-plus package to 0.98.0 version. You can do it with following commands:

cd ~/.atom/packages
rm -rf vim-mode-plus
apm install vim-mode-plus@0.98.0

[jackcasey]

FYI a matching issue for vmp: https://github.com/t9md/atom-vim-mode-plus/issues/860

[t9md]

Could reproduce, and if I comment out following keymap, issue disappear.

'g ^': 'vim-mode-plus:move-to-first-character-of-screen-line'

This is super similar with https://github.com/atom/atom-keymap/issues/195 which I fixed long time ago.

As my understanding this is not vmp issue. And as my impression, this is not atom-keymap issue.

I don't know how proton handle keymap uniquely, can you share what proton is doing in space x x like keyboard interaction?

[dvcrn]

When hitting space in VIM normal mode, proton attaches the class proton-mode to the workspace and removes vim-mode and vim-mode-plus. When the class .proton-mode is set, all keystrokes will be captured (here)

Internally, when proton starts, a big tree of keybindings is generated. Like {x: {x: "action"}}. Each keystroke when .proton-mode is active traverses this tree until one leaf node is found.

[t9md]

Internally, when proton starts, a big tree of keybindings is generated. Like {x: {x: "action"}}. Each keystroke when .proton-mode is active traverses this tree until one leaf node is found.

That include the fallback keymap which is used when proton's unique keymap did not match?

I'm imaging like this

• space g s is handled by proton, but space g x is matched by user's keymap.cson but it dispatched proton's auto-generated keymap initialized at proton starts.

Anyway I thinks position where error thrown is here.

https://github.com/atom/atom-keymap/blob/3b17ede7f10e5a9c5097b495a03e88e2c7bacfa3/src/partial-keyup-matcher.js#L36

Can you imagine situation this can happen?

[jackcasey]

With t9md's help we boiled the repro steps down simply adding to a key mapping like:

# keymap.cson
'atom-text-editor.vim-mode-plus:not(.insert-mode)':
  'g ^': 'vim-mode-plus:move-to-first-character-of-screen-line'

This is enough to cause the same broken behaviour. Even when the vim-mode-plus package has been removed (and the vmp class manually added to the editor (atom.workspace.getActiveTextEditor().element.classList.add("vim-mode-plus", "normal-mode"))

Also if you substitute f for g then it's the SPC f that breaks things. So it's not 'g' dependant. Purely the sequence style keybinding ending with a ^.

Thanks very much for the help t9md.

[dvcrn]

The thing though is, proton is removing the vim-mode-plus class when you hit space, so this shouldn't even trigger anymore.

I added

'atom-text-editor.vim-mode-plus:not(.insert-mode)':
  'g ^': 'vim-mode-plus:move-to-first-character-of-screen-line'

to my keymap but don't run into these problems

Which packages do you have installed?

[jackcasey]

Ok I just reproed this again :) (MacOS fyi...) Here are the steps I took which should be explicit and reproducible:

~ $ export ATOM_HOME=~/.atom_testing
~ $ mv ~/.proton ~/.proton_backup
~ $ atom

• Manually add proton-mode package via [ Preferences / Install ] UI and let it finish installing
• Quit atom
• Run the following in the earlier terminal window (With ATOM_HOME set)

~ $ apm remove vim-mode-plus
~ $ vim /Users/jack/.proton (add vim-mode-plus to disabled packages with separate editor to be sure)

• restart atom (making sure to do so in the terminal with the explicit ATOM_HOME
• open a file in atom
• note that pressing space now just enters a space into the buffer no menu is shown
• run the following js on the atom debug JS console to fake the vim mode plus classes:

atom.workspace.getActiveTextEditor().element.classList.add("vim-mode-plus", "normal-mode") 

• note that spacebar now brings up the proton menu
• try SPC g s etc
• it works fine
• add a keybind to keymap.cson ending in ^ such as:

  'atom-text-editor.vim-mode-plus:not(.insert-mode)':
  'g ^': 'vim-mode-plus:move-to-first-character-of-screen-line'

• now go back to the same file and try SPC g
• get exception!
• confirm in [ Preferences / Packages ] that vim-mode-plus is not installed

Hopefully that works for others. I've tried to be as careful and explicit as possible.

[boogie666]

hello,

@jackcasey but it does look like it has something to do with the '^' in the binding.

This is likely due to the fact that atom-keymap uses the ^ internaly to check for 'keyup' for some wierd reason, instead of passing an object around...

it looks like this is the problem

https://github.com/atom/atom-keymap/blob/master/src/key-binding.coffee#L80

if you step though the code you see that it basically interprets ^ as the user wanting to bind to 'keyup' instead of the ^ (i.e shift+6) key...

As far as i can tell the only way to fix this is either for vim-mode-plus to fix it (see below) or have the atom team fix it...

vim-mode-plus can chage the keybinding from "g ^" to "g shift+6", since basically all plugins that have any keybinding that has a '^' somewhere would have to change the ^ to shfit+6, so this is more of a long term workaround imo 😃

in the mean time if you add this to ~/.atom/init.coffee

init.coffee

it will prevent this error and maintain the same behaviour for vim-mode-plus. until a propper fix turns up.

also be sure to not have anything bound to 'g ^' anyware (keymaps.cson or .proton)

Hope this helps :)

Cheers, Boogie.

[jackcasey]

Thanks for the extra diagnosis and great workaround!

So it seems like the right fix would be to change that line in atom keymap from:

bindingRemainderContainsOnlyKeyups = false unless bindingKeystroke.startsWith('^')

to:

bindingRemainderContainsOnlyKeyups = false unless bindingKeystroke.startsWith('^') && bindingKeystroke.length > 1

I'll see if I have time today to figurue out a failing test for in atom-keymap and do a PR into there.

Cheers, Jack

[jackcasey]

FYI: https://github.com/atom/atom-keymap/pull/221/files

[jackcasey]

Not sure it actually fixes the issue for us entirely, just getting set up to properly test dev atom-core packages...

[t9md]

Although I haven't check detail of PR. I want to share my stance and understanding of this issue.

• This issue doesn't happen with vim-mode-plus + Atom combination
• This issue only appear with Atom + proton + vim-mode-plus
• vim-mode-plus do nothing special for keymap
  • It just added g ^ default keymap in v0.99.0
  • Atom have no problem to handle this keymap(had problem in older version but already fixed).

From the above fact, what I can say is, proton do something special for keymap handling which cause this issue.

Even if atom-keymap have some space to fix, "what behavior of proton's keymap handling cause this issue" is first thing I want to know.

[jackcasey]

There are definitely still problems in atom-keymap around ^ being presumed to only be used as a keyup prefix.

But yes, it would be best to figure out the exact causes. I think it's around proton deciding if there is a partial match pending or not while it navigates through its own 'version' of sequence mappings (with the nested menus).

I'm having trouble running atom in dev mode with a custom version of atom-keymap :/ I have it in ~/.atom/dev/packages but doesn't seem to be getting used.

[jackcasey]

OIC atom-keymap isn't an apm module. So need to do this kind of stuff: https://discuss.atom.io/t/how-to-setup-development-for-core-modules-like-atom-keymap/9221/18

[t9md]

I couldn't remember correctly how I did it before.

If you want to use locally changed atom-core module. You need to build atom-core locally. Then npm link which purpose is your atom-build dir's node_module/atom-keymap is symlinked to your custom atom-keymap. Launch atom with dev-mode, also specify ATOM_RESOURCE_PATH.

http://flight-manual.atom.io/behind-atom/sections/developing-node-modules/

[jackcasey]

I can't get atom working in develop mode and using a custom version of atom-keymap at the moment :/ (compiled library node version mismatches!) and am not going to be able to look at this for a a week or so. I feel like my PR to atom-keymap would probably fix the issue, but I can't be sure without testing. Someone should feel free to take over from me next week if they like, on both this issue and my PR on atom-keymap

[Ben3eeE]

@t9md The correct environment variable is ATOM_DEV_RESOURCE_PATH you forgot the DEV part :slightly_smiling_face:

@jackcasey The steps in the flight-manual for linking node modules does not always work, you may have to run apm rebuild inside your clone of atom/atom instead of in the atom-keymap folder.

[jackcasey]

Thanks @Ben3eeE That did it! I think it must have been the only thing I didn't explicitly try. (I've been running election-rebuild and trying to completely rebuild and repackage atom etc...

So I've tested with that change to atom-keymap and it does fix this issue. I'm still not sure why proton-mode causes it to manifest. My theory, that is unsubstantiated, is that to do the 'proton menu' proton hooks into or otherwise causes atom-keymap to check if there is a partial keymap in progress, and when the trailing ^ is hit it was incorrectly saying that no there was a full keymap hit that is just waiting on the keyup. Pretty vague I know...

So anyway, like I said I'm not able to work on this from tomorrow so I'll leave it at that for now. I'll ping the PR on atom-keymap because I think it's a proper / correct bugfix regardless of anything else (the very specific failing unit test attests to that). If anyone wants to figure out all the inner workings in the meantime feel free :)

[Ben3eeE]

:+1: I'll check it out as soon as I have the chance to. I'm also wondering why it only manifests in proton-mode.

[Ben3eeE]

Hey,

We have just merged https://github.com/atom/atom-keymap/pull/221 and the fix is currently scheduled to be released in Atom 1.22.

Thanks @jackcasey for opening the PR and everyone for the detailed information 💯

[gemantzu]

Any info on when 1.22 will be available?

Edit: Nevermind, I just saw that 1.22.0-beta0 is up

[jackcasey]

This issue is fixed in atom-keymap. So I'll just close it :)