Allow multi key remappings in config file

[sudormrfbin]

Allows configuration like this:

[keys.normal.g.l]
"d" = "goto_definition"  # gld
"r" = "goto_reference"  # glr

[keys.normal.z]
"$" = "goto_file_end"  # z$

[keys.insert.j]
"k" = "normal_mode"  # jk

Or alternatively in inline table syntax:

[keys.normal]
# gld -> definitions, glr -> references
"g" = {"l" = {"d" = "goto_definition", "r" = "goto_reference"} }
"z" = {"$" = "goto_file_end"}

[keys.insert]
"j" = {"k" = "normal_mode"}

We can't use something like "jk" = "normal_mode" since that will collide with key names like "backspace" and "esc".

Key mappings are represented in a tree like structure, which can be useful in the future to dynamically show infobox contents (currently the keys are hardcoded, and user mappings cannot be shown).

[kirawi]

So I presume you would do this for 3 keys?:
"e" = { "s" = { "c" = "normal_mode" } }

[sudormrfbin]

So I presume you would do this for 3 keys?:
"e" = { "s" = { "c" = "normal_mode" } }

@kirawi Exactly, it can be arbitrarily nested to any level. An alternative syntax (supported by the toml spec):

[keys.insert.e.s]
"c" = "normal_mode"

[pickfire]

[keys.insert.e.s]
"c" = "normal_mode"

This does seemed better, is it possible to switch to this? Either way, the current configuration mechanism is just temporary.

[sudormrfbin]

Both methods are supported by the parser, it's in the defined toml spec :)

[pickfire]

I think it is better to integrate this with infobox to kill two birds with one stone.

• able to change infobox keys
• able to show infobox for user custom keys

But with that we need additional information in the settings.

Maybe you can look at how infobox is implemented, I have this in mind so I parse the keys the same way the config is currently being parsed.

helix/helix-term/src/commands.rs

Lines 3691 to 3714 in e2bcef7

	mode_info! {
	/// space mode
	space_mode, SPACE_MODE,
	/// file picker
	"f" => file_picker,
	/// buffer picker
	"b" => buffer_picker,
	/// symbol picker
	"s" => symbol_picker,
	/// window mode
	"w" => window_mode,
	/// yank joined to clipboard
	"y" => yank_joined_to_clipboard,
	/// yank main selection to clipboard
	"Y" => yank_main_selection_to_clipboard,
	/// paste system clipboard after selections
	"p" => paste_clipboard_after,
	/// paste system clipboard before selections
	"P" => paste_clipboard_before,
	/// replace selections with clipboard
	"R" => replace_selections_with_clipboard,
	/// keep primary selection
	"space" => keep_primary_selection,
	}

Do you want to give it a try? If anything can ping me.

[sudormrfbin]

The infobox was one of the reasons I went with a tree to represent the keys since it would be easy to get the next set of possible keys after a keypress; this is how I was thinking of doing it:

--- a/helix-term/src/keymap.rs
+++ b/helix-term/src/keymap.rs
@@ -195,7 +195,18 @@ impl Default for Keymaps {
             ctrl!('o') => KeyCommand(Command::jump_backward),
             // ctrl!('s') KeyCommand(ommand)::save_selection,
 
-            key!(' ') => KeyCommand(Command::space_mode),
+            key!(' ') => SubKeymap(hashmap!(
+                key!('f') => KeyCommand(Command::file_picker),
+                key!('b') => KeyCommand(Command::buffer_picker),
+                key!('s') => KeyCommand(Command::symbol_picker),
+                key!('w') => KeyCommand(Command::window_mode),
+                key!('y') => KeyCommand(Command::yank_joined_to_clipboard),
+                key!('Y') => KeyCommand(Command::yank_main_selection_to_clipboard),
+                key!('p') => KeyCommand(Command::paste_clipboard_after),
+                key!('P') => KeyCommand(Command::paste_clipboard_before),
+                key!('R') => KeyCommand(Command::replace_selections_with_clipboard),
+                key!(' ') => KeyCommand(Command::keep_primary_selection)
+            )),

So instead of using the mode_info macro (which we would have to treat as a special case since it does it's own key handling waiting for the next key), all the multi key mappings are defined in keymap.rs. We can now handle user mappings and default mappings the same way when showing the infobox (user mappings will properly override them too). The descriptions can be stored as a field the Command struct along with the name and callback function. I'll try playing around with it :)

[pickfire]

I don't think tree is the issue, I think both specifying the message during compile-time works but I think we should reuse the mode_info! or have something similar to replace the current mechanism, that is able to make it more ergonomics from our side.

But there is a benefit to let user specify their message, because we don't know how the user structure their keys so it is better to let users override the description for the infobox as well, which is why I think the description should be in runtime config, because we don't know how user want to structure the stuff, and the infobox should depends on how it is structured to have custom text.

Although infobox integration is not necessary now but at the very least I would like to see the replacement of key!() to use our own parser, so that we can specify "space" and the rest like the infobox macro.

[archseer]

I think this is a good feature, but it shouldn't be bolted onto the current data structures.

We should implement a custom trie structure for keys, and then fetching the value either returns a final node (a command) or a node with more branches, which can then be used to render the autoinfo in a generic way for any subcommand.

[archseer]

Example from iota https://github.com/gchp/iota/blob/8f35dd5cba2f07077590619760e62dbb74a71fab/src/iota/keymap.rs

[sudormrfbin]

@pickfire I think we can start with non-configurable infobox descriptions since the user cannot make custom commands yet, and if the default descriptions need changing they should probably be changed in the repo itself. I'll look into replacing key! (and the whole keymap definition method) with another macro which doesn't look so crowded.

@archseer You mean storing the state within Keymaps like in iota and removing it from EditorView ? That does seem cleaner, also a proper trie will represent it better 👍

[archseer]

That said, I took a look and the trie in iota looks a lot like what we have now (a bunch of hashmaps) :) so we can probably just tweak the current implementation.

Yeah I think the way to go is improve the Keymap type so that we can define the mode mappings there without the need for custom commands. You can still store the state (keypresses up to now) inside the EditorView, basically you push the key onto the keys vec, traverse and see what you get. If it's a command you can clear the keys and execute, if it's a branch you keep the keys and render an infobox instead.

In order for the descriptions to work we'll have to define them per command (instead of within the mode! macro) but that can be done similar to how we do it for :commands.

[cessen]

This PR is heading in a direction that I'm really excited about, but I think can potentially go even further. I'm not sure if we want to tackle this in this PR, but here's what I was thinking:

The keymap tree wouldn't just handle sequential key presses, but also modes. Basically, if you think of normal mode as being the "root", then the keymap tree branches off from there, and that tree can have both:

• Transient nodes: used for sequential key combos, like this PR is currently enabling. These nodes can optionally include names for displaying in the info box pop-up.
• Sticky/mode nodes: used for defining custom editor modes. When you press a key corresponding to a sticky node, you're there permanently until you explicitly escape out of it. These nodes would include a name for displaying in the status line (e.g. SEL), as well as a key that can be used in themes.

This would allow nearly complete customization of Helix's interaction model.

The main question I'm still unsure of with the above idea is how insert mode fits into all of this. Possible each sticky node has a boolean flag that specifies whether it's an "insert-like" mode or a "normal-like" mode. Not sure.

(Sorry if this is off-topic for the PR... I'm just really excited about getting the Helix interaction model super customizable, ha ha.)

[pickfire]

I think we can just defer infobox integration but I would like to see hashmap! macro being updated to add the Leaf.

[sudormrfbin]

Goto line number doesn't work now since there is no separate function for goto mode. Adding a function for goto mode only will require treating it as a special case especially for handling the infobox and user remappings. A solution is to add it to gg (keybind would change from 82g to 82gg, like in vim).

[pickfire]

Can't we add hooks?

[sudormrfbin]

But that would make it impossible to have any other key under g to take a count, and the user might remap some function to it that takes a count.

[pickfire]

The keys under g from what I see all does not need a count.

[sudormrfbin]

The default ones don't, I meant the case as a user remapping some other function that takes count to a key under g. I will probably do this myself with ge to be vim-like (go to end of previous word) and it would take a count.

[sudormrfbin]

• Sticky/mode nodes: used for defining custom editor modes. When you press a key corresponding to a sticky node, you're there permanently until you explicitly escape out of it. These nodes would include a name for displaying in the status line (e.g. SEL), as well as a key that can be used in themes.

@cessen That's a neat idea; I suppose it's similar to emacs major modes (I don't use emacs so not sure) ?

I think we can just defer infobox integration but I would like to see hashmap! macro being updated to add the Leaf.

@pickfire I moved the keys from mode_info! definitions into keymap! so the keys work but infobox doesn't; I'll add the infobox integration soon.

Yeah I think the way to go is improve the Keymap type so that we can define the mode mappings there without the need for custom commands. You can still store the state (keypresses up to now) inside the EditorView, basically you push the key onto the keys vec, traverse and see what you get. If it's a command you can clear the keys and execute, if it's a branch you keep the keys and render an infobox instead.

I made Keymap itself stateful instead of deferring to EditorView, and infobox is todo.

[cessen]

@cessen That's a neat idea; I suppose it's similar to emacs major modes (I don't use emacs so not sure) ?

I don't use emacs either, so no idea, ha ha. I just have a hope for modes themselves to also be fully customizable in Helix, and that's one idea for how to accomplish that in a relatively coherent way. But maybe another approach would be better.

[archseer]

Great work! I think this is looking much nicer than what we have on master, especially the keymap! macro.

• No default keys assigned for signature help and expand selection (based on tree sitter node)

Yeah signature help is incomplete and will likely be auto triggered by a hook when a trigger character is used (defined by lsp, usually ( then retriggered on , when typing out arguments).

Expand selection used to be [ but was replaced. I think we want [o and ]o for contract & expand.

[archseer]

Tested it out on local, it works! Just a few things:

• We should use a BTreeMap to preserve the order of commands in the popup. We grouped them logically before so that goto start of file and end of file sit next to each other on the list.
• The " mode" suffix on each header isn't needed, I'd just use "Goto"/"Window" etc
• Each line in the goto popup now starts with goto. I was thinking an easy hack would be: do all the key entries in the mode start with the mode name? If so, remove the prefix. That way it will show as "implementation" in the goto mode, but "Goto implementation" if it's in a user defined mode. I'm not too opinionated here, we might skip this. It's nice because it keeps the popup small though.

[pickfire]

Can you please reuse the old titles that you removed rather than creating new ones? Like the following.

Suggested change

	"g" => { "Goto mode"
	/// goto
	///
	/// When specified with a count, it will go to that line without entering the mode.
	"g" => {

The extra description will only be shown in the rustdoc but the "goto" word will be shown in both rustdoc and infobox.

[sudormrfbin]

Reusing the old titles seem better since they are shorter 👍🏾. But specifying it as a doc comment and then having an additional description would be hard to parse since the macro is recursive (also writing descriptions for rustdoc seem redundant since they are not user facing and only developers will see it)

[pickfire]

Maybe I will take a look at this later myself.

[pickfire]

But can you please keep them as normal comments here? So later I can just change it.

[sudormrfbin]

I changed the descriptions themselves to use the ones from mode_info in ad7e412

[pickfire]

I think the docs and macro can be improved. "m" => { "Match mode" looks weird. But at least for now the docs should take from the old one for those that was writtten, I tuned them specifically for the infobox.

[sudormrfbin]

Great work! I think this is looking much nicer than what we have on master, especially the keymap! macro.

Thank you ! Making the macro recursive was quite fun :)

• We should use a BTreeMap to preserve the order of commands in the popup. We grouped them logically before so that goto start of file and end of file sit next to each other on the list.

@archseer BTreeMap sorts itself by keys, so it won't remember insertion order. I think we can keep a private Vec that tracks the order the keys inserted and then use it when constructing the infobox.

• Each line in the goto popup now starts with goto. I was thinking an easy hack would be: do all the key entries in the mode start with the mode name? If so, remove the prefix. That way it will show as "implementation" in the goto mode, but "Goto implementation" if it's in a user defined mode. I'm not too opinionated here, we might skip this. It's nice because it keeps the popup small though.

I'll try a hand at it :)

I think the docs and macro can be improved. "m" => { "Match mode" looks weird.

@pickfire My initial preference for the syntax was "m" => "Match mode" { and then doc comments, but the recursive nature of the macro (for example everything after => has to be a catch all metavar like tt so as to match both leaves and nodes) makes it hard to write a readable macro using those syntaxes.

[archseer]

BTreeMap sorts itself by keys, so it won't remember insertion order.

Ah right! I guess we'd need https://github.com/bluss/indexmap but I'm not sure if it's worth pulling in another dependency, a Vec of keys to sort by would work yeah.

[sudormrfbin]

BTreeMap sorts itself by keys, so it won't remember insertion order.

Ah right! I guess we'd need bluss/indexmap but I'm not sure if it's worth pulling in another dependency, a Vec of keys to sort by would work yeah.

Yeah pulling a dependency seems overkill, I went ahead and added it with a Vec :)

[archseer]

Great work!

[blaggacao]

@sudormrfbin Would it be possible to serialze the default keymap from a runtime/default-keymap.toml?

I need to rebind almost all keys because I'm on a workman layout sometimes, so this is really annoying to do as a partial rhs merge since you always miss one key somewhare that causes then odd behavior, later.

With a runtime/default-keymap.toml, I could just copy that over and through editing discipline guarantee some decent consistency.

Edit: to keep the select mergeing with normal, it ought to be:

runtime/default-normal-mode-keymap.toml
runtime/default-select-mode-keymap-override.toml
runtime/default-insert-mode-keymap.toml

[cessen]

I need to rebind almost all keys because I'm on a workman layout sometimes

Even aside from keyboard layouts, I think it would be good to support starting from a blank slate anyway, for building totally custom keymaps.

[sudormrfbin]

@blaggacao Moving the default keys to runtime/ will render the editor completely unusable if the runtime folder is not installed properly which I suppose is not that much of a problem since it's part of the install procedure. But since helix is not packaged for most distros right now it might lead to a lot of confusion (we regularly have people dropping in to the matrix channel reporting problems with themes and highlighting that are mostly caused by not installing the runtime properly but the experience is not as jarring as having no keybindings).

Other than that I don't see a problem with it, though using scancodes as mentioned in #165 and #133 would be a more elegant solution to the problem.