Added support for ctrl_h and ctrl_i in keymap config

[nuvw]

I ran into this problem while trying to define a key binding for ctrl_h. Neither
did it work, nor did I get any error or warning. A quick search in the source
code revealed that the corresponding key strokes are represented as Key::Backspace
whereas the key binding itself is represented as Key::Ctrl('h'). I changed the
latter to be represented as Key::Backspace too, which fixes the issue. I included
a similar fix for ctrl_i (and also ctrl_[).

[osa1]

Thanks for the patch.

On xterm and alacritty, ctrl-h generates 8, and backspace generates 127. However in term_input these are both mapped to Key::Backspace:

tiny/crates/term_input/src/lib.rs

Lines 181 to 182 in 73f2401

	[8] => Key::Backspace,
	[127] => Key::Backspace,

I don't remember why this is. In my testing just now, it seems like only 127 should be mapped to backspace. I tried with xterm too. We should fix this.

As for ctri-i and tab, they unfortunately generate the same byte which is 9. So you can't distinguish them. Instead of mapping ctrl-i to an action in your keymap, you should map tab to the same action. You can then trigger that action with both ctrl-i and tab (as you can't distinguish them in a terminal app).

[nuvw]

Thanks for the reply and your great work on this tiny piece of software!

There seems to be quite some history regarding ASCII 127 (called DEL and often generated by the backspace key) and ASCII 8 (called BS which, curiously enough, stands for backspace and is generated by Ctrl-H). To add to the confusion, on many terminals the backspace key seems to generate 127 while ctrl-backspace generates 8. According to my reading, however, this does not seem to be handled consistently across all terminals; but I did not manage to find a very clear source on this issue.

As it stands, I think it would still be reasonable to have 8 and 127 map to the same default action of, well, deleting a character. But, having it done as it is now in term_input/src/lib.rs makes a more fine-grained reconfiguration impossible, unnecessarily so in my opinion. I think it should be part of the default configuration in libtiny_tui/src/key_map.rs.

Regarding ctrl_i and tab: I understand that it is not possible to distinguish the two. Still, to me it seems wrong to have the possibility of defining a key binding by typing ctrl_i in the configuration file, which is then simply and silently ignored in all cases.

[osa1]

Regarding ctrl_i and tab: I understand that it is not possible to distinguish the two. Still, to me it seems wrong to have the possibility of defining a key binding by typing ctrl_i in the configuration file, which is then simply and silently ignored in all cases.

Right, I see the problem now. I think this patch is probably OK to fix this, but I also think this kind of thing should still be done with help from term_input. Until this change libtiny_tui didn't know that ctrl-i and tab generate the same bytes and term_input cannot distinguish the two, or that term_input generates tab for this byte sequence, instead of ctrl-i.

Since term_input deals with terminal input and needs to know about the byte sequences, in principle this kind of thing would be done in term_input as well to have all the intricacies dealt with in one place.

Some ideas:

• term_input could export a set of non-alphanumeric character keys that it generates, and we could check user key bindings only use those keys. This set could then either have tab or ctrl-i (but not both), and tiny would print an error message when you try to map ctrl-i to an action saying that that key is never generated.

  Unless the error says which key to use instead or whether the key is not supported at all, this is not that helpful for the user who wants to use ctrl-i.

  Telling the user which key to use instead would mean finding all the ctrl keys and others that generate the same sequences and having a map in term_input saying which key should be instead, for a given unsupported key. I don't know if we could get this information from an termcap-like thing or whether we have to manually generate them. Manually generating them would be a lot of work probably and difficult to update too.

• term_input could generate a set of keys (instead of one key) for each press. The set would mostly have one key, except in keys like tab, for which it would generate {tab, ctrl-i}.

  This also requires finding all the keys that generate the same sequences, so share some of the same problems as above.

• Merge this PR, add more cases mapping ctrl-char to other keys as necessary (e.g. when a user finds another case).

I think (3) is probably fine for now as other solutions require investing quite a bit into this, and I don't have the time currently.

However we should at least add comments before this code explaining why we map these ctrl-keys to other keys.

I also don't remember what kind of tests we have and whether this would be easy to test, but if you could add tests for these mappings (optional) that would make sure we won't accidentally break this behavior in the future as we change the code.

(The CI failure on macos is probably #439, an existing issue in a dependency that appeared after a dependency bump. It's potentially a serious issue on macos build, it would be great if a someone who uses macos could take a look 😄 )

I guess it's fine to keep mapping 8 and 127 to Backspace for now.

[osa1]

Thanks!