Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Key representation in bindings #11

Open
massimiliano-mantione opened this issue May 1, 2022 · 2 comments
Open

Key representation in bindings #11

massimiliano-mantione opened this issue May 1, 2022 · 2 comments

Comments

@massimiliano-mantione
Copy link

The representation of modifiers in the Key enum is probably not powerful enough and partially non-coherent.

Special characters (it seems non-printable control characters from the unicode control plane) are handled with specific enum branches (Key::Left, Key::Home, Key::Insert...) while "regular" characters are handled by the Key::Char branch.

The partial inconsistency is due to the fact that it would be possible to create a Key::Char(c) where c has the value of the ASCII LEFT, and it would be different from Key::Left but should have the same semantics.
This is a bit strange but not bad in practice.
However, it becomes a problem if one wanted to create a Key representing the combination of a modifier and a control character.
For instance, if I wanted to represent CTRL-LEFT I would probably have to create a Key::Ctrl(k) where k is the "LEFT" ASCII char value, which is not obvious (and I wonder if it would actually work).

The "expressive power" problem comes from the impossibility to combine modifiers (there's no way to specify CTRL-ALT-LEFT), and from the lack of a "SHIFT" modifier.
For instance, "SHIFT-RIGHT" could increase the selection size of one character in a text editor with CUA-like keybindings, but I don't know how to represent this with the Key type.

Would it be possible to change the Key enum to be a struct, logically like this:

struct ModifiedKey {
  pub key: Key,
  pub shift: bool,
  pub control: bool,
  pub alt: bool,
  pub super: bool, // do we need this? do terminals support it?
}

where Key is an enum just like the current one, but without the Ctrl and Alt branches?

This struct, with no special optimization, would be a bit larger than 64 bits, which is unfortunate.
I can imagine several alternative representations that would fit into 64 bits but would make matching less ergonomic.
But before proposing them, is the problem I am describing real, or am I missing something?

How could I handle CTRL-SHIFT-RIGHT in the current version of zi?
I would need it to represent a key binding to "extend the selection by one word" in zee :-)

@massimiliano-mantione
Copy link
Author

As a side note, it would be nice to have Key implement both Display and TryFrom<&str>, so that implementing serialization to and from strings becomes trivial (and there would be a single, "canonical" way of doing it).
This would help in working on issue 28 in zee :-)
I could provide a PR for this very quickly, but I would like to fix this issue before implementing Display and TryFrom<&str>.
And I am not providing a PR for this issue right away because it would be a breaking change and I would like to discuss it first...

@massimiliano-mantione
Copy link
Author

massimiliano-mantione commented May 3, 2022

I had a deeper look at the code, and IMHO Key should be just like KeyEvent in crossterm, and the function map_key should do nothing.
I realize this would be a deeply breaking change for every project using zi, but I see no other way of representing events like CTRL-SHIFT-RIGHT.

I see two ways forward (well, three...)

  1. do nothing (but I would not be able to use zi, and therefore zee)
  2. simply do the breaking change
  3. expose KeyEvent in zee without an actual breaking change (keeping also the current Key type)

I would find option 3 confusing, but the idea would be to:

  • expose KeyEvent (pub use it)
  • inside KeyMap and DynamicBindings, use KeyEvent
  • inside Bindings, expose APIs to build them both in terms of KeyEvent and Key values, transforming KeyEvents into Keys when needed

Perhaps this would allow exposing KeyEvent without a breaking change.
Or perhaps not because I am missing something :-)
But it would keep the technical debt of the double representation.

I would prefer option 2 and I am willing to produce a PR for it.
But, if needed, I could also write a PR for option 3.
Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant