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

The Key enum: categories, extension, commands (shortcuts) #2995

Closed
dhardy opened this issue Aug 2, 2023 · 9 comments
Closed

The Key enum: categories, extension, commands (shortcuts) #2995

dhardy opened this issue Aug 2, 2023 · 9 comments

Comments

@dhardy
Copy link
Contributor

dhardy commented Aug 2, 2023

This is partly beyond the scope of Winit but deserves a little discussion. See also #1806 and the many prior discussions (too many to easily navigate). Also pyfisch/keyboard-types#19

Key replaces the old VirtualKeyCode as a listing of named keys. The main difference, besides Key being much larger and based on a different standard is that Key includes Character(Str), Unidentified(NativeKey) and Dead(Option<char>) super-categories.

Categories

The W3C standard categorises "named keys" into:

  1. Special keys: "unidentified"
  2. Modifier keys: Alt, NumLock, ...
  3. Whitespace keys (note: Key includes Space; W3C standard does not)
  4. Navigation keys: arrows, home/end, page up/down
  5. Editing keys: backspace, undo, paste, ..
  6. UI keys: find, help, escape, ...
  7. Device keys: power, brightness, ..
  8. IME and composition keys
  9. General-purpose function keys: F1-12 (note: Key includes up to F35), Soft1-4
  10. Multimedia keys
  11. Multimedia numpad keys
  12. Audio keys
  13. Speech keys
  14. Application keys
  15. Browser keys
  16. Mobile phone keys
  17. TV keys
  18. Media controller keys

Omissions

This is a big list with a lot of categories, yet still omits some things:

  • Key::Character: typed text
  • Key::Dead: usually the start of a key-combo sequence
  • Text editing commands requiring multiple keys: ViewUp / ViewDown, WordLeft / WordRight, Italic, Bold, Underline, Link
  • Search: Find is included (under UI keys) but FindNext, FindPrevious, FindReplace are not
  • Fullscreen
  • Remap displays (the key laptops use to manage how to use external displays: clone, extended desktop etc.)

Related: shortcut mapping

Somehow an app needs to match combos like Ctrl+C (which could be mapped to Key::Copy) and Ctrl+Left / Alt+Left / Command+Left (which could be mapped to Key::WordLeft if this were added).

So far, I have been using a different enum for this, Command. The result: much repetition and repetitive matching, yet with many omissions.

As an alternative, I could use a much reduced Command enum like this:

enum Command {
    Key(winit::keyboard::Key),
    ViewUp,
    WordLeft,
    FindNext,
    // ...
}

Caveat: if e.g. FindNext were in the future added to Key, this enum would need a breaking change.
Caveat: values like Command::Key(Key::Find) have long names making matching tedious.

Questions

Should Key categorise its values? If so, some values could be shared with the PhysicalKey enum, and some key mapping operations may be easier, but there isn't a large incentive to make changes here. (And a dis-incentive: matching e.g. Key::Multimedia(MultimediaKey::Open) is tedious with non-obvious category.)

Should, instead, all "named keys" be placed under another NamedKey enum?

Should Key be extended to include things like WordLeft, FindNext, Fullscreen? These are not known keys (though they almost could be), but are common UI functionality and shortcut targets.

@daxpedda
Copy link
Member

daxpedda commented Aug 4, 2023

I like the idea of categorizing the Key enum, but I agree that there isn't a large incentive to do so.
The big advantage to me is that it would make it easier for users to match against specific actions they want to make, e.g. application will probably handle basic keys (letters, numbers, etc.) with completely different code then actions (multimedia, clipboard, etc.). You already mentioned the tradeoffs this comes with.

Supporting shortcuts would be a worthwhile addition imo and I'm in favor of adding that. This might be an incentive to add categorization instead of either adding commands to Key, which might or might not make sense in certain cases, or having a different type, which would not be nice to use considering the overlap with Key.

All in all I'm not particularly in favor of categorizing Key, unless in combination with adding shortcut support, which I am in favor of.

@kchibisov
Copy link
Member

@daxpedda you usually handle them together though, because you have a general trigger_binding(key, modifiers) catch all, and then if you haven't triggered anything you process the rest, so having them together is nice.

Should Key be extended to include things like WordLeft, FindNext, Fullscreen? These are not known keys (though they almost could be), but are common UI functionality and shortcut targets.

No, it shouldn't, it a subject to an application to handle that, winit can't decide what combination could possibly mean.

Probably having a Named Category to reduce duplication in both enums is fine, however, we certainly will have values in present only physical key enum, like keypad keys.

@madsmtm
Copy link
Member

madsmtm commented Aug 4, 2023

Noting that we want to integrate with keyboard-types at some point. (So any change we do should be considered in the idea of sending it upstream as well).

@dhardy
Copy link
Contributor Author

dhardy commented Aug 4, 2023

True. There don't appear to be any recent changes to keyboard-types, but we should CC @pyfisch.

@pyfisch
Copy link

pyfisch commented Aug 12, 2023

I am not following in detail how winit is progressing, however these are my 2 cents.

For matching shortcuts I suggest you have a look at this simple matcher, which is part of keyboard-types: https://docs.rs/keyboard-types/latest/keyboard_types/struct.ShortcutMatcher.html

Should Key categorise its values? If so, some values could be shared with the PhysicalKey enum

You should absolutely not share values between Key and PhysicalKey enums. The values in PhysicalKey just designate a position on the keyboard but layouts can and do use it for a very diffferent purpose. I think of the names just as mnemonics because its easier to remember "LeftShift" than "first key in the third row".

@kchibisov
Copy link
Member

Yeah, I agree. There's no reason to merge the physical keys with normal keys, they don't make any sense.

We don't do any shortcuts in winit, downstream clients do that with all the freedom they have.

@dhardy
Copy link
Contributor Author

dhardy commented Aug 15, 2023

Some of my questions are resolved:

  1. Should Key categorise its values? No.
  2. Should, instead, all "named keys" be placed under another NamedKey enum?
  3. Should Key be extended to include things like WordLeft, FindNext, Fullscreen? No; such categories should be application-defined.

To return to question 2, being able to define an app-specific enum Command which is a super-set of Key without Character / Unidentified / Dead variants would be useful to me. This would of course require adapting both keyboard-types and winit.

If we agree I am happy to make PRs, otherwise this issue can be closed.

@pyfisch's ShortcutMatcher is orthogonal to this need; if used it would require T = Command.

@kchibisov
Copy link
Member

Should, instead, all "named keys" be placed under another NamedKey enum?

I'm not sure about that one tbh, but I could see a desire to do things like that. The main issue is that it could complicate matching, though, not that much...

@dhardy
Copy link
Contributor Author

dhardy commented Oct 14, 2023

Additional motivation: I wish to cleanly write a custom serialiser for Key. Moving most variants out to something like this is not necessary for this use-case, but allows cleaner code:

pub enum Key<Str = SmolStr> {
    Action(Action),
    Character(Str),
    Unidentified(NativeKey),
    Dead(Option<char>),
}

The main issue is that it could complicate matching, though, not that much...

Especially given that matching will mostly be concerned with things like Key::Character('c') which don't change...

@dhardy dhardy mentioned this issue Oct 14, 2023
11 tasks
kchibisov added a commit to kchibisov/winit that referenced this issue Oct 21, 2023
Split `Key` into clear categories, like `Named`, `Dead`, Character`, `Unidentified`
removing the `#[non_exhaustive]` from the `Key` itself.

Similar action was done for the `KeyCode`.

Fixes: rust-windowing#2995
Co-authored-by: Kirill Chibisov <[email protected]>
kchibisov added a commit that referenced this issue Oct 21, 2023
Split `Key` into clear categories, like `Named`, `Dead`, Character`, `Unidentified`
removing the `#[non_exhaustive]` from the `Key` itself.

Similar action was done for the `KeyCode`.

Fixes: #2995
Co-authored-by: Kirill Chibisov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants