Add a custom "when clause context" for "waiting for input"

[mtsknn]

This PR adds a custom when clause context to check if amVim is waiting for input. For example, g is an incomplete input, waiting for another keystroke, e.g. another g (so the complete input would be g g).

This allows specifying more powerful keyboard shortcuts in VS Code. For example, both #21 and #184 would be fixed (at least partially) with this PR and the following keyboard shortcuts:

[
  {
    "key": "j",
    "command": "cursorMove",
    "when": "editorTextFocus && amVim.mode == 'NORMAL' && !amVim.waitingForInput",
    "args": { "to": "down", "by": "wrappedLine" }
  },
  {
    "key": "k",
    "command": "cursorMove",
    "when": "editorTextFocus && amVim.mode == 'NORMAL' && !amVim.waitingForInput",
    "args": { "to": "up", "by": "wrappedLine" }
  }
]

No tests though. 🥲

Some more context (pun intended): setContext is already used in Dispatcher.ts for amVim.mode and in Configuration.ts for these three:

• amVim.configuration.shouldBindCtrlCommands
• amVim.configuration.shouldMimicVimSearchBehavior
• amVim.configuration.shouldUseVimStyleNavigationInListView

---

Edit (Nov 12, 2022): Changed amVim.inputs from string/array (e.g. [] or d,3) to amVim.waitingForInput (boolean).

[alisonatwork]

This looks like a clever workaround! I don't have time to review and test more carefully at the moment, but hopefully I'll have time to give it a longer look on the weekend. My one concern is if we are overlooking something that perhaps another person uses. There have been one or two changes we made that seemed to work great to solve one problem but ended up causing a regression for other people due to the many different ways we all like to use vi keys. But if we don't change the default keybindings and allow people to set it themselves if they prefer... seems like a great option. Thanks!

[mtsknn]

Agree that the default keybindings shouldn't be changed. This PR doesn't change anything else either, just adds a new thing, so it shouldn't break anything (fingers crossed 😄).

[alisonatwork]

I've had a longer look at the problems this is trying to solve now, and I do like the functionality, if it works as expected. It seems sensible to provide a way for the user to choose to "short circuit" some of the existing motions if they prefer instead to have a different behavior. I will check out this branch and do some testing to make sure it's okay.

In the mean time, would you like to have a look at #305 and give it a bit of a test? I spent a bit of time today implementing gk and gj as proper motions. They work with counts (5gk) and actions (dgk), so should cover most of the use cases people will want for manipulating these wrapped lines in more complex ways.

[alisonatwork]

Okay, so I've checked out this branch and had a play around with the feature now and it works as advertized 💃 Thanks again for the contribution!

I have some questions about what you think should happen in different scenarios, though.

For the case where amVim.inputs == '[]', this seems to be a useful context. You can successfully bind keys to do things in VS Code that should only do things when they aren't prefixed by other keys. For example 3j will still go down 3 amVim lines, while j can be overridden to only go down one wrapped line. So, it makes sense and it is useful to expose the empty inputs case.

But what about the case where amVim.inputs == '3' or amVim.inputs == 'd,3'? How can we make use of this context?

Thinking about soon-to-be-implemented command gm (goto middle of screen line), you would hope that you could implement a rudimentary version of it like this:

{
  "key": "m",
  "command": "cursorMove",
  "args": { "to": "wrappedLineColumnCenter" }
  "when": "editorTextFocus && amVim.mode != 'INSERT' && amVim.inputs == 'g'",
},

And... it works! Except the problem is that g remains dangling in the inputs. What I would expect to happen is for the inputs to clear once the command is executed.

How you could do it is by installing a secondary macros extension like https://github.com/ctf0/macros that knows how to chain together commands, then have the keypress first trigger a cursorMove, then type escape... But that feels a bit brittle, because escape might not do the same thing in every mode.

I feel like there are several paths forward for this.

1. We only expose a boolean context that indicates if the input is waiting. This way you could do something like "when": "!amVim.inputWaiting" and not ever worry about what the actual content of the inputs is.
2. We continue to expose the current list of inputs, but we also expose an action that macro extensions can use to (re)set the inputs. So you could potentially build a macro that completes partially-implemented commands for your use case.
3. We expose a new amVim action that explicitly exists to both run a VS Code command and reset the inputs.

The benefit of option 1 is that it is clean and easy to implement, and it will be immediately useful for the use case of people who want keys like hjkl to move the cursor in a different way to amVim standard way, while not breaking traditional motions for those keys. The disadvantage is that users will not be able to implement longer commands on their own.

The benefit of option 2 is that it would allow users to implement some longer commands on their own. The disadvantage is that it essentially will only be useful for users who have installed another extension. My suspicion is that users who have installed a macros extension anyway are going to be able to figure out some other workaround for their use case.

The benefit of option 3 is that we can much more strictly control the situation where a VS Code command is triggered based on input context. It will behave how people expect it to behave: the input matcher will be cleared, and only one command will be run. The disadvantage is again that we are adding more complexity to the amVim code to handle corner cases for users to self-implement incomplete commands that perhaps would be better to just implement into amVim directly.

The fourth option is do nothing and merge this PR as-is.

Personally I am learning toward option 1, because I don't see a lot of people wanting to try construct their own multi-key commands, even if we did provide functionality to support it in theory. However there is a very clear use case identified by several tickets on this repo where people want to be able to move the cursor in different ways without breaking motions.

What do you think @mtsknn?

[alisonatwork]

@mtsknn have you had time to think about my previous comment? Do you have a strong opinion on which solution to go with?

@karlhorky I know you have done a workaround for getting hjkl to work across folds, do you have a preference for any one of these solutions over the other?

My current feeling is that the easiest to understand solution is simply a boolean context of true or false to indicate if the inputs are waiting or not.

[mtsknn]

@alisonatwork Very good thoughts, thanks!

I agree that option 1 is the best for now. I'll update the PR soon.

Option 2 or 3 could be implemented later if needed. Maybe option 3 so we wouldn't rely on another extension, but on the other hand it would add more complexity to the amVim code like you said. I would leave this decision to the future.

---

By the way, in my current PR description, empty inputs should be === '' instead of === '[]'. I mistakenly used the latter because I first tried to set the array as-is to the context, i.e.:

// Current PR
commands.executeCommand('setContext', 'amVim.inputs', inputs.join(','));

// Previous attempt (before publishing the PR)
commands.executeCommand('setContext', 'amVim.inputs', inputs);

That however had some problems IIRC so I changed to stringifying the array.

Edit: nope. 🤦‍♂️ I forgot that I'm setting empty arrays as '[]' to the context (I now wonder why) so never mind. 😅

[mtsknn]

All right, I updated the PR and did quick tests and my use case seems to still be working. ✌️ I opted for amVim.waitingForInput instead of amVim.inputWaiting since I like to name boolean names like that (*ing at the beginning), I think that's clearer. (Though I didn't check how other boolean variables are named in the amVim code, whoops.)

Thanks for your patience, @alisonatwork. 😁

[alisonatwork]

Thanks for the update @mtsknn - I like your new variable name too. I will merge this now and will push a new release next week.

[alisonatwork]

Sorry for the delay on releasing this, @mtsknn, I was waiting to see if Microsoft would be able to clean up my screw-up that caused a bunch of the history to be deleted from the VS Code Marketplace, but since it's been a month I am guessing not. Let me know how it goes in 1.36.0 and thanks again for the contribution!

[mtsknn]

Thanks for the heads-up! Works great with a quick test (using the two keybindings from the PR description). 😍