cancel operator-pending on unrecognized key strokes

[jacekkopecky]

(edited) Fix #734 and #680. This is an alternative to #781, because of issues noted below in #764 (comment) and further comments. Discussion welcome.

This fix overrides Atom's normal key binding resolution process for multi-keystroke key bindings: where Atom will do ci3w like this:

1. c -> change
2. i -> some potential bindings, e.g. iw, ip etc., so let's wait
3. 3 -> i3 is not known and no key binding starts with i3 so let's break it down
   1. i on its own is unknown in operator-pending mode, so let's ignore it
   2. let's try again 3 -> a prefix, good
4. w combines with 3 so the final result is c3w

What we want is

1. c -> change
2. i as above
3. 3 leads to i3 which is unknown and cancels c
4. w moves cursor to next word

We could beep on a cancellation like this.

The atom.keymaps.cancelPendingState() call is a private API, but I can't judge whether that's too bad for this PR.

[bronson]

I had a problem with this PR in vim-mode-next. dd didn't work anymore (just ignored). Might be an issue with the way I merged it?

Here's the bad tree: https://github.com/bronson/vim-mode-next/commits/no-dd

And here's the revert that fixes it: bronson/vim-mode-next@6105cea

[jacekkopecky]

@bronson I can't replicate it. Are specs failing, or is it interactive behaviour? Do you have anything with d in your own keymap, by any chance?

[bronson]

No specs failing, I emptied my keymap. I'll try disabling extensions...

Yep! It only happens when vim-surround is installed. That makes sense. I should have tried disabling extensions before reporting. Sorry, getting lazy. :)

[bronson]

Hm, vim-surround is starting to look notorious: #657 (comment)

I wish I could mark packages as conflicting... I don't want to break people using both vim-mode-next and vim-surround, but I don't want vim-surround's nasty keymap handling to hold up progress either... Dunno.

edit: ok, maybe it's not so nasty. :)

[jacekkopecky]

Oh boy, this complicates things. Vim-surround has a key binding for d s, so vim-mode's d d becomes a binding atom fails to match. Let's see if vim-surround's maintainers are receptive to rethinking their key binding approach.

[jacekkopecky]

No, I can't seem to formulate this as an issue against vim-surround - it's making a valid use of atom's key binding infrastructure, after all. The above commit addresses these particular interactions, but I'm no longer sure that this approach to handling unrecognised commands is appropriate in Atom vim-mode.

How about alternatively I work on showing the commands awaiting completion and if a user types vix or ci3w and gets bitten, they have u or b to help, and they'll learn that's not a valid combination.

Maybe a debounced beep on unrecognized single-key bindings would help. I'll look into that too.

[bronson]

Yeah, too bad this can't just be pinned on vim-surround. Would have made things easy! :)

You don't think it makes sense to just rework vim-surround's mappings to allow this, even though what they're doing now is OK? If it makes things work and keeps the code clean then that seems reasonable...

[jacekkopecky]

Thinking about your comments @bronson made me realize that perhaps #764 should only cancel on single-key unrecognized key bindings. 386e0c8 implements that.

Now this PR implements the following behaviour:

• ci3w behaves as 3w after canceling c because of unrecognized i in operator-pending mode (replayed by Atom after matching unrecognized i3). ci3 beeps and means 3.
• cqw behaves as w after canceling c because of unrecognized q. cq beeps and is a no-op.
• vix behaves as vx, but it beeps on the unrecognized ix (because atom replays i which goes unrecognized, and then it replays x).

I also plan to expand the specs to spy on beeping, so apm test doesn't beep and the specs verify that the beeps happen.

Thoughts welcome.

[jacekkopecky]

@bronson with the current #764, the interaction with vim-surround looks like this:

1. you press d, Atom sees the possible ds from vim-surround so waits for a bit in case you press something. Vim-mode doesn't know yet that anything is happening.
2. if you wait, Atom will time out waiting for ds and just play d, which puts us in the operator-pending mode. Vim-surround's complex key bindings cause a delay before the user sees operator-pending mode (and the associated cursor change).
3. pressing d quickly after the first d, before Atom times out as described before, means Atom fails to find a multi-keystroke binding that starts with dd so it replays d, that's recognized by vim-mode which goes operator-pending, and then Atom replays the second d immediately and vim-mode deletes the line.

In short, waiting for complex key strokes delays user feedback, and that can be an issue for vim-surround. I'm tired now but will open it tomorrow if you don't do it first. 8-)

[lee-dohm]

As stated in the README, this package is no longer maintained and is deprecated. We recommend that people use the vim-mode-plus package instead. Because of this, we are archiving this repository and closing all issues and pull requests. Thanks very much for your support and contributions!