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

feat: Add "win" as an alias for both lwin and rwin #934

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Miitto
Copy link
Contributor

@Miitto Miitto commented Jan 21, 2025

Adds an alias called win for both lwin and rwin, for some parity with other modifiers, and convenience.

For any keybind that uses win, it duplicates it and makes on for lwin and one for rwin.

- commands: ["focus --direction left"]
  bindings: ["win+left"]
- commands: ["focus --direction right"]
  bindings: ["win+right"]

Will be parsed and stored in the ParsedConfig as if you had used:

- commands: ["focus --direction left"]
  bindings: ["lwin+left", "rwin+left"]
- commands: ["focus --direction right"]
  bindings: ["lwin+right", "rwin+right"]

@@ -137,10 +137,19 @@ impl KeyboardHook {

for keybinding in keybindings {
for binding in &keybinding.bindings {
let mut duplicate_with_rwin = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the ideal here would be to instead have a Key enum and implement conversion methods from strings and from vkeys. That way we could convert from strings when creating the hashmap in keybindings_by_trigger_key, and then from the u16 vkey when the keypress comes in. We could then fully avoid duplicating the keybinding and also handle any other vkeys that don't line up nicely

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum Key {
    A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, X, Y, Z,
    D0, D1, D2, D3, D4, D5, D6, D7, D8, D9,
    Numpad0, Numpad1, Numpad2, Numpad3, Numpad4, Numpad5, Numpad6, Numpad7, Numpad8, Numpad9,
    F1, F2, F3, F4, F5, F6, F7, F8, F9, F10, F11, F12,
    F13, F14, F15, F16, F17, F18, F19, F20, F21, F22, F23, F24,
    Shift, LShift, RShift,
    Control, LControl, RControl,
    Alt, LAlt, RAlt,
    Win, LWin, RWin, // !! win is a separate member of the enum
    Space, Escape, Back, Tab, Enter,
    Left, Right, Up, Down,
    NumLock, ScrollLock, CapsLock,
    PageUp, PageDown, Insert, Delete, End, Home,
    PrintScreen,
    Multiply, Add, Subtract, Decimal, Divide,
    VolumeUp, VolumeDown, VolumeMute,
    MediaNextTrack, MediaPrevTrack, MediaStop, MediaPlayPause,
    OemSemicolon, OemQuestion, OemTilde, OemOpenBrackets,
    OemPipe, OemCloseBrackets, OemQuotes, OemPlus,
    OemComma, OemMinus, OemPeriod,
    Custom(u16) // !!
}

impl Key {
    pub fn from_str(key: &str) -> Option<Self> {
        match key.to_lowercase().as_str() {
            "a" => Some(Key::A),
            "b" => Some(Key::B),
            // ...
            _ => {
              // Check if the key exists on the current keyboard layout.
              let utf16_key = key.encode_utf16().next()?;
             // ...
             Some(Key::Custom(xx))
        }
    }

    pub fn from_vk(vk: u16) -> Option<Self> {
        match VIRTUAL_KEY(vk) {
            VK_A => Some(Key::A),
            VK_B => Some(Key::B),
            // ...
        }
    }
}

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

Successfully merging this pull request may close these issues.

2 participants