Behavior choice: Helix keymap

[Strackeror]

Hey there !

So I've been maintaining a personal use fork of dance with helix keybindings for a while now, but I've been seeing some of the keybinds making their way into the original codebase (with plans for more in #323), so I thought I'd just go ahead and try to remake it in a way that could fit into the main project.

So, here is a draft for a 'behavior' choice, where a user could choose which base keymap to use from a single setting. This is currently just the basic normal and select mode keymaps, without any of the helix menus, but it's a good starting point to show how it would look like in practice.

Currently the handling in the commands file is pretty naive, with a lot of duplication for the select mode. And I'm not sure on the backward compatibility when adding a new condition to a lot of keybindings like that, so I'm very open to discussion.

[71]

Hey there, and thanks for the PR! I haven't looked at everything, but I overall agree with the idea!

I don't want to let the perfect be the enemy of the good, but one desire that I had when thinking about letting users switch between Kakoune/Dance/Helix keybindings is supporting multiple keymaps (e.g. non-QWERTY, macOS). Given the large number of keybindings in Dance I would have preferred to change the user's keybindings.json file automatically or to distribute alternative keybindings as extensions, though nothing beats the "behavior" as introduced by your PR in terms of convenience, so for now I'm perfectly fine with keeping things as-is.

Anyway, it is still good to have a "library" of Helix-specific keybindings; even if the way these keybindings are chosen changes in the future, such a change would be easy to automate following this change. But for now I think I would prefer marking the "behavior" setting as "experimental".

I'm also wondering if there might be an overlap between custom modes and behaviors (e.g. maybe instead of behaviors, we could have helix/normal and kakoune/normal modes).

And I'm not sure on the backward compatibility when adding a new condition to a lot of keybindings like that, so I'm very open to discussion.

This is a good point, I will think about it as well.

[Strackeror]

I... actually didn't think of generating a new extension with just keybinding and settings. I'll explore that idea, it feels like it'd be a better way to organize and avoid bloating the package.json further.

I'm not a fan of editing keybindings.json, mostly because I like to keep the vscode config files somewhat simple, it can very easily become a mess to manage with settings sync, profiles, the different configuration levels... It's a big reason I kept maintaining an actual fork of dance instead of just config files :)

The more I think about it, the more I think just having 'namespaced' helix modes and keeping the current kakoune modes as default is probably the least likely to break anything, and also the most simple to handle. Makes it a bit harder to show it as 'experimental' though, since you'd just change the default mode.

[Strackeror]

Okay, so, turns out making a basic extension with a bunch of keybinds and setting overrides was pretty easy !

It seems to work pretty well, with a couple caveats :

• The extension contribute setting for overriding default configurations can't merge or do anything complex, which leads to some duplication between the package builders. I didn't think it was worth factoring that duplication yet, but my mind could easily be changed.

• To avoid having to ignore all the keybinds in the base extension, I went ahead with the namespaced modes idea. It's currently very basic and just some basic string replacement when switching modes and generating keybinds, so that could probably be improved, but i'm not sure on the exact way yet.

Apart from that, I like it a lot better as a starting point. It's a bit barebones and missing some helix behaviors, but it's probably a better idea to defer those to separate PRs.

[Strackeror]

Thanks a lot! Sorry again for the delay.

I haven't looked at everything though I did skim over the files (and left a few comments, though they're primarily nits / code style).

Seeing the helix: select commands inline like that while knowing that they won't "bloat" the extension is pretty cool!

I think I'm mostly worried about code duplication; I think that I'd like to get to a point where Dance is Dance "Core" + Dance "Kakoune" (or Dance "Helix"), so we would benefit from reusing the same code/assets/package.json build machinery for all extensions.

Yah, no worries, there's no hurry, and this thing's become a sizeable PR 😅.

I'll have to think about how to deduplicate the package generating code, I'm not sure exactly which part would benefit from abstracting yet. I'm frankly not sure what a 'Core' dance would include. Right now in the keybind definitions, "core" more of a catchall for helix+kakoune. Using it like that would result in this very weird and incomplete intersection of the two, and I don't think that's the ideal way to go.

[71]

I'll have to think about how to deduplicate the package generating code

If you'd like I can take care of this; we could submit your PR soon (either to master or to some other branch, releasing the Helix extension as "pre-release" only or waiting to release it) and I'll look into deduplicating some of the stuff.

I'm frankly not sure what a 'Core' dance would include

For me, it would include all the Dance commands and API, but none of the keybindings. That way you would install the "Kakoune keybindings" or "Helix keybindings", both of which would depend on "Dance Core", and you would only get the keybindings you need.

With this model, I guess we would create a new extension "Dance Core", and migrate the current extension to be "Dance with Kakoune keybindings".

Right now in the keybind definitions, "core" more of a catchall for helix+kakoune.

"Dance Core" (the extension) would be different from core keybindings; core keybindings are more "common" or "shared" keybindings which would be included in both extensions.

[Strackeror]

If you'd like I can take care of this; we could submit your PR soon (either to master or to some other branch, releasing the Helix extension as "pre-release" only or waiting to release it) and I'll look into deduplicating some of the stuff.

I'd be okay with that, if that works for you ! It'll also allow me to submit the couple other helix-related PRs I'm working on :)

"Dance Core" (the extension) would be different from core keybindings; core keybindings are more "common" or "shared" keybindings which would be included in both extensions.

Ah I see, that makes a lot more sense when thinking of it this way. Might make sense to rename the 'core' category to avoid confusion in that case, but that's a small issue.

[akdor1154]

hey guys, where's this sitting? any way an excited bystander can help get this over the line? :)

[daedroza]

Any updates to this?

[71]

Hey all. I'm sincerely sorry for the delay; I had some minor anxiety looking at GitHub notifications for the past ~6 months, but am going to try to get this submitted soon.

Before I merge this PR, can someone please help with the following:

• [ ] Review the PR, preferably someone with experience with Helix, and who has submitted code to Dance before. Not asking to be nitpicky, "Works, LGTM" would be enough.
• [ ] Write instructions on how to use the Helix extension. I'm mostly worried about conflicts between Dance "Main" and Dance "Helix" defaults. Similarly, "Works without any additional configuration" would be enough.

[71]

I'll try to daily-drive this for a few days.

My keybindings with "when": "editorTextFocus && dance.mode == 'normal'" don't work when I'm in helix/normal. This is somewhat expected, but I wonder if it would be worth having something like dance.mode == '*/normal' or something in the future. Maybe the default Dance modes are in dance/normal, and we add a new setting dance.defaultModeNamespace (set to helix in the Helix extension)? Doesn't need to be fixed in this PR though.

[Strackeror]

Oh boy, been a while since there's been some activity in here :) When clauses have a match operator, so you could technically already go for dance.mode =~ /^(.+\/)?normal$/ or something of the sort. It doesn't feel super clean though, maybe we could add some other when clause variables for separate namespace and modes ?

It is somewhat incomplete in terms of all the commands that are available, however I personally am not bothered by that; I wouldn't want to attempt to achieve perfection before merging this, I'd much rather have the basics in place and settled. It's easy enough to add (and contribute!) any glaring omissions (btw - i have 80% of a match menu :) ).

This branch is definitely meant as a first step for other helix integrations down the line. I have a much more complete reproduction of helix behavior down in my spacemenu branch, including space, match, goto and bracket menus, and much more complete treesitter integrations. The plan is to progressively clean up and PR some of those into the main repo :)

[kabouzeid]

gg doesn't go to start of file and gi should be gs (both fixed here: https://github.com/Strackeror/dance/pull/3)

mm shouldn't start a selection but only jump. also it should go to the enclosing pair, not the next pair to the right.

mim and mam or missing completely.

[kabouzeid]

Would it make sense to include the space, window, ] and debug menus as well, or is it out of scope? I currently have them in my personal config.

[71]

Agreed, "chasing perfection" is the main reason why I couldn't open GH notifications for several months, so even if it's incomplete I'd be happy to merge the PR. Just want to make sure it's a good basis for future changes.

@kabouzeid These will likely be useful (thanks for the input!), but won't block this PR from being submitted first.

[kabouzeid]

I'm not sure how much sense it makes to have the Helix "helper" extension. As it stands, the main extension defines the keybindings, while the helper extension only adds the modes and menus. I don’t see an easy way to move the keybindings to the helper extension as well.

Given that the two can't be cleanly separated, it might make more sense to keep everything in the main extension rather than adding complexity.

The user can still easily enable Helix mode with a single config option: dance.defaultMode: 'helix/normal'.

[kabouzeid]

Also just noticed, currently the registers for yank, delete etc are set to the system clipboard. Should be the * register.

[71]

@kabouzeid I agree, ideally they would also be part of the same extension but I'm worried about scaling: the Dance extension registers ~300 keybindings, and adding an Helix mode (then later different keybindings for macOS, then different keybindings for different keyboard layouts like AZERTY) could easily bump this to >500 keybindings. These bloat views like "Keyboard Shortcuts", and I haven't benchmarked anything but it's possible VS Code is not happy to evaluate the "when" clause for hundreds of keybindings when the mode is changed.

That said I'm open to ideas which would allow us to keep a single extension without significant drawbacks (or benchmarks which prove the drawbacks mentioned above aren't that bad).

[cloudhan]

How about stop worrying about the imperfection and just merge it, make a preview release and see how it goes. It is not a mission critical piece of software that once pushed to a planet, which is a hundred million miles away from the Earth, cannot be reverted. VSCode made it quite easy to switch between versions.

[71]

@cloudhan Unfortunately the extension doesn't currently build. I proposed a solution in https://github.com/Strackeror/dance/pull/2 but as pointed out by the author of this PR, it would be better to have different files rather than symlinks. I will tackle it when I get some time, but it may not happen for a while (so, PRs welcome).

Edit: I submitted https://github.com/Strackeror/dance/pull/4 to fix this.

[cloudhan]

@71 On Windows 11, the only problem I encountered is error: unknown option '--follow-symlinks' and remove all those flags builds the extension flawlessly.

EDIT: not yet, seem vsce is dispatch in a wrong folder, aka, not from dir where package.json is located, but from the project root.

[cloudhan]

Here is some build artifact on Windows dance-0.5.15-pre1.vsix.zip dance-helix-keybindings-0.5.15-pre1.vsix.zip extract and install if someone want to test it out.

[71]

Thank you all again for bearing with me here, especially @Strackeror! Sorry again for the delay. We now have a strong base to build on, so future changes should be far simpler to merge.

[bezbac]

I've been using Dance with a custom helix-like set of keybindings for more than a year now, and I just wanted to thank everybody involved in getting this PR merged! The quality of this extension is exceptionally high, and by integrating helix bindings, I hope it will see additional adoption.

Thanks maintaining it. Really amazing work! 🫶

[akdor1154]

Awesome stuff guys, thanks heaps :)

[kabouzeid]

Awesome, thanks! Let's refine and add the missing functionality with focused PRs that can be merged quickly :)

[Strackeror]

Ayyy, it's done ! Glad this is getting out there, more improvements coming soon :)

[lukepighetti]

hey all, is there any documentation showing how to use these? I'm on pre-release and can't seem to find any settings for enabling helix mode

[ongyx]

Hi @lukepighetti, the Helix keybindings haven't been released to the VSCode Marketplace yet, so your best bet is to build from source assuming you have NodeJS installed:

git clone https://github.com/71/dance
cd dance/extensions/helix
npm i
npm i -g yarn  # for vsce
npm run package

This produces a VSIX file, which automatically enables Helix keybindings once installed in VSCode.

I've built a copy from the master branch at commit 4e3d259 if you'd like the extension without building it yourself: dance-helix-keybindings-0.1.0.vsix.zip Just rename the extension back to .vsix, then you can press Ctrl+Shift+P -> Extensions: Install from VSIX... to use it.

[merlinaudio]

WHOA

Insane work guys. Thank you for making this possible

[71]

Hey all,

First, appreciate you packaging the extension @ongyx. Although in general, .vsix files can be pretty dangerous (as VS Code extensions can do pretty much anything on your computer)[^1], so I'd advise anyone that uses it to open it up first (it's a .zip file with a different extension) and review the code (there shouldn't be much outside of package.json).

Secondly, I'm trying to publish the extension to the store but have hit a few issues while making sure it works. Notably, after loading the Helix extension the normal Dance modes disappeared, but that doesn't happen all the time. That is, helix/normal exists, but normal doesn't. Another problem is that the command palette only displays Dance commands on normal mode, not on helix/normal (edit: fixed in f3117fd84cae1e94f28cad119aa8f2004c342c47).

Edit: It looks like it is intended for configurationDefaults (used by the Helix extension to add Helix modes to Dance) to overwrite, not contribute modes: https://github.com/microsoft/vscode/issues/168498. I'm not sure why Dance sometimes receives configurations that are merged instead (verified via the debugger and vscode.workspace.getConfiguration). I think we have several ways to fix this:

1. Instead of contributing modes via dance.modes, we use a JS API where we can properly contribute modes instead of overriding them. The downside being that now the Helix extension doesn't simply have a package.json file, it also has some script.
2. We completely overwrite the insert and normal modes instead of adding alternatives called helix/normal and helix/insert. I think this would also remove some confusion over which mode is currently loaded, which mode needs to be configured in the settings, and which mode keybindings apply to, so this might actually be better this way.
   • But thinking about it, if we use the same mode names then the default keybindings given by Dance will be the same, so we'll need a way of disabling them somehow.
3. We modify the Helix extension so that it contributes both default Dance modes and its own modes. That's an easy fix, but feels hacky, especially if we consider future versions of Dance and Helix that have slightly different default keybindings.
4. We manually load the default modes in Dance and merge them with the ones given to us by VS Code. This should similarly be easy to do, but similarly feels hacky to me.
5. We split Dance into Dance Core and Dance; Dance Core doesn't have builtin keybindings or modes, just commands needed for them.

[^1]: Although I understand it is partially my fault since I didn't publish the extension on the store.