From 2e62f161494c479e8520f38e7775583ddb6d6f7f Mon Sep 17 00:00:00 2001 From: tidely <43219534+tidely@users.noreply.github.com> Date: Mon, 26 May 2025 16:34:04 +0300 Subject: [PATCH] gpui: Apply cfg at compile time (#31428) - Use compile time `cfg` macro instead of a runtime check - Use `Modifiers` instead of a bunch of `bool` when parsing a `Keystroke`. Release Notes: - N/A --- crates/gpui/src/platform/keystroke.rs | 136 +++++++++++++------------- 1 file changed, 66 insertions(+), 70 deletions(-) diff --git a/crates/gpui/src/platform/keystroke.rs b/crates/gpui/src/platform/keystroke.rs index 765e8c43be..3a7070da7f 100644 --- a/crates/gpui/src/platform/keystroke.rs +++ b/crates/gpui/src/platform/keystroke.rs @@ -94,37 +94,33 @@ impl Keystroke { /// secondary means "cmd" on macOS and "ctrl" on other platforms /// when matching a key with an key_char set will be matched without it. pub fn parse(source: &str) -> std::result::Result { - let mut control = false; - let mut alt = false; - let mut shift = false; - let mut platform = false; - let mut function = false; + let mut modifiers = Modifiers::none(); let mut key = None; let mut key_char = None; let mut components = source.split('-').peekable(); while let Some(component) = components.next() { if component.eq_ignore_ascii_case("ctrl") { - control = true; + modifiers.control = true; continue; } if component.eq_ignore_ascii_case("alt") { - alt = true; + modifiers.alt = true; continue; } if component.eq_ignore_ascii_case("shift") { - shift = true; + modifiers.shift = true; continue; } if component.eq_ignore_ascii_case("fn") { - function = true; + modifiers.function = true; continue; } if component.eq_ignore_ascii_case("secondary") { if cfg!(target_os = "macos") { - platform = true; + modifiers.platform = true; } else { - control = true; + modifiers.control = true; }; continue; } @@ -134,7 +130,7 @@ impl Keystroke { || component.eq_ignore_ascii_case("win"); if is_platform { - platform = true; + modifiers.platform = true; continue; } @@ -158,7 +154,7 @@ impl Keystroke { if component.len() == 1 && component.as_bytes()[0].is_ascii_uppercase() { // Convert to shift + lowercase char - shift = true; + modifiers.shift = true; key_str.make_ascii_lowercase(); } else { // convert ascii chars to lowercase so that named keys like "tab" and "enter" @@ -170,37 +166,30 @@ impl Keystroke { // Allow for the user to specify a keystroke modifier as the key itself // This sets the `key` to the modifier, and disables the modifier - if key.is_none() { - if shift { - key = Some("shift".to_string()); - shift = false; - } else if control { - key = Some("control".to_string()); - control = false; - } else if alt { - key = Some("alt".to_string()); - alt = false; - } else if platform { - key = Some("platform".to_string()); - platform = false; - } else if function { - key = Some("function".to_string()); - function = false; + key = key.or_else(|| { + use std::mem; + // std::mem::take clears bool incase its true + if mem::take(&mut modifiers.shift) { + Some("shift".to_string()) + } else if mem::take(&mut modifiers.control) { + Some("control".to_string()) + } else if mem::take(&mut modifiers.alt) { + Some("alt".to_string()) + } else if mem::take(&mut modifiers.platform) { + Some("platform".to_string()) + } else if mem::take(&mut modifiers.function) { + Some("function".to_string()) + } else { + None } - } + }); let key = key.ok_or_else(|| InvalidKeystrokeError { keystroke: source.to_owned(), })?; Ok(Keystroke { - modifiers: Modifiers { - control, - alt, - shift, - platform, - function, - }, + modifiers, key, key_char, }) @@ -331,18 +320,18 @@ fn is_printable_key(key: &str) -> bool { impl std::fmt::Display for Keystroke { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { if self.modifiers.control { - if cfg!(target_os = "macos") { - f.write_char('^')?; - } else { - write!(f, "ctrl-")?; - } + #[cfg(target_os = "macos")] + f.write_char('^')?; + + #[cfg(not(target_os = "macos"))] + write!(f, "ctrl-")?; } if self.modifiers.alt { - if cfg!(target_os = "macos") { - f.write_char('⌥')?; - } else { - write!(f, "alt-")?; - } + #[cfg(target_os = "macos")] + f.write_char('⌥')?; + + #[cfg(not(target_os = "macos"))] + write!(f, "alt-")?; } if self.modifiers.platform { #[cfg(target_os = "macos")] @@ -355,31 +344,38 @@ impl std::fmt::Display for Keystroke { f.write_char('⊞')?; } if self.modifiers.shift { - if cfg!(target_os = "macos") { - f.write_char('⇧')?; - } else { - write!(f, "shift-")?; - } + #[cfg(target_os = "macos")] + f.write_char('⇧')?; + + #[cfg(not(target_os = "macos"))] + write!(f, "shift-")?; } let key = match self.key.as_str() { - "backspace" if cfg!(target_os = "macos") => '⌫', - "up" if cfg!(target_os = "macos") => '↑', - "down" if cfg!(target_os = "macos") => '↓', - "left" if cfg!(target_os = "macos") => '←', - "right" if cfg!(target_os = "macos") => '→', - "tab" if cfg!(target_os = "macos") => '⇥', - "escape" if cfg!(target_os = "macos") => '⎋', - "shift" if cfg!(target_os = "macos") => '⇧', - "control" if cfg!(target_os = "macos") => '⌃', - "alt" if cfg!(target_os = "macos") => '⌥', - "platform" if cfg!(target_os = "macos") => '⌘', - key => { - if key.len() == 1 { - key.chars().next().unwrap().to_ascii_uppercase() - } else { - return f.write_str(key); - } - } + #[cfg(target_os = "macos")] + "backspace" => '⌫', + #[cfg(target_os = "macos")] + "up" => '↑', + #[cfg(target_os = "macos")] + "down" => '↓', + #[cfg(target_os = "macos")] + "left" => '←', + #[cfg(target_os = "macos")] + "right" => '→', + #[cfg(target_os = "macos")] + "tab" => '⇥', + #[cfg(target_os = "macos")] + "escape" => '⎋', + #[cfg(target_os = "macos")] + "shift" => '⇧', + #[cfg(target_os = "macos")] + "control" => '⌃', + #[cfg(target_os = "macos")] + "alt" => '⌥', + #[cfg(target_os = "macos")] + "platform" => '⌘', + + key if key.len() == 1 => key.chars().next().unwrap().to_ascii_uppercase(), + key => return f.write_str(key), }; f.write_char(key) }