diff options
| -rw-r--r-- | src/config/src/utils/get_input.rs | 105 | ||||
| -rw-r--r-- | src/core/src/components/confirm/mod.rs | 1 | ||||
| -rw-r--r-- | src/core/src/components/shared/editable_line.rs | 7 | ||||
| -rw-r--r-- | src/input/src/event.rs | 7 | ||||
| -rw-r--r-- | src/input/src/key_bindings.rs | 12 | ||||
| -rw-r--r-- | src/input/src/key_event.rs | 72 |
6 files changed, 131 insertions, 73 deletions
diff --git a/src/config/src/utils/get_input.rs b/src/config/src/utils/get_input.rs index 1c77765..1205b26 100644 --- a/src/config/src/utils/get_input.rs +++ b/src/config/src/utils/get_input.rs @@ -9,8 +9,8 @@ pub(crate) fn get_input(config: Option<&Config>, name: &str, default: &str) -> R for mut value in input.split_whitespace().map(String::from) { let mut modifiers = vec![]; - if let Some(index) = value.to_lowercase().find("shift+") { - modifiers.push("Shift"); + let shift_index = value.to_lowercase().find("shift+"); + if let Some(index) = shift_index { value.replace_range(index..index + 6, ""); } if let Some(index) = value.to_lowercase().find("control+") { @@ -22,45 +22,53 @@ pub(crate) fn get_input(config: Option<&Config>, name: &str, default: &str) -> R value.replace_range(index..index + 4, ""); } - values.push(format!( - "{}{}", - modifiers.join(""), - match value.to_lowercase().as_ref() { - "backspace" => String::from("Backspace"), - "backtab" => String::from("BackTab"), - "delete" => String::from("Delete"), - "down" => String::from("Down"), - "end" => String::from("End"), - "enter" => String::from("Enter"), - "esc" => String::from("Esc"), - "home" => String::from("Home"), - "insert" => String::from("Insert"), - "left" => String::from("Left"), - "pagedown" => String::from("PageDown"), - "pageup" => String::from("PageUp"), - "right" => String::from("Right"), - "tab" => String::from("Tab"), - "up" => String::from("Up"), - v => { - if v.len() > 1 { - // allow F{number} values - if v.starts_with('f') && v[1..].parse::<u8>().is_ok() { - v.to_uppercase() - } - else { - return Err(ConfigError::new( - name, - input.as_str(), - ConfigErrorCause::InvalidKeyBinding, - )); - } - } - else { - value - } - }, + let mut key = match value.to_lowercase().as_ref() { + "backspace" => String::from("Backspace"), + "backtab" => String::from("BackTab"), + "delete" => String::from("Delete"), + "down" => String::from("Down"), + "end" => String::from("End"), + "enter" => String::from("Enter"), + "esc" => String::from("Esc"), + "home" => String::from("Home"), + "insert" => String::from("Insert"), + "left" => String::from("Left"), + "pagedown" => String::from("PageDown"), + "pageup" => String::from("PageUp"), + "right" => String::from("Right"), + "tab" => String::from("Tab"), + "up" => String::from("Up"), + v => { + let v_len = v.chars().count(); + // allow F{number} values + if v_len > 1 && v.starts_with('f') && v[1..].parse::<u8>().is_ok() { + v.to_uppercase() + } + else if v_len == 1 { + value + } + else { + return Err(ConfigError::new( + name, + input.as_str(), + ConfigErrorCause::InvalidKeyBinding, + )); + } + }, + }; + + // Shift support was partially removed, due to Shift not being universally reported, but still maintain + // some backwards compatibility with printable characters + if shift_index.is_some() { + if key.len() == 1 { + key = key.to_uppercase(); + } + else { + modifiers.push("Shift"); } - )); + } + + values.push(format!("{}{}", modifiers.join(""), key)); } Ok(values) } @@ -75,7 +83,9 @@ mod tests { use crate::testutils::{invalid_utf, with_git_config}; #[rstest] - #[case::single("a", "a")] + #[case::single_lower("a", "a")] + #[case::single_upper("A", "A")] + #[case::single_non_ascii("ẞ", "ẞ")] #[case::backspace("backspace", "Backspace")] #[case::backtab("backtab", "BackTab")] #[case::delete("delete", "Delete")] @@ -97,16 +107,19 @@ mod tests { #[case::modifier_character_uppercase("Control+A", "ControlA")] #[case::modifier_character_number("Control+1", "Control1")] #[case::modifier_character_special("Control++", "Control+")] + #[case::modifier_character_non_ascii("Control+ẞ", "Controlẞ")] #[case::modifier_character("Control+a", "Controla")] #[case::modifier_special("Control+End", "ControlEnd")] #[case::modifier_function("Control+F32", "ControlF32")] - #[case::modifier_control_alt_shift_lowercase("alt+shift+control+end", "ShiftControlAltEnd")] - #[case::modifier_control_alt_shift_mixedcase("aLt+shIft+conTrol+eNd", "ShiftControlAltEnd")] - #[case::modifier_control_alt_shift_out_of_order_1("Alt+Shift+Control+End", "ShiftControlAltEnd")] - #[case::modifier_control_alt_shift_out_of_order_2("Shift+Control+Alt+End", "ShiftControlAltEnd")] + #[case::modifier_control_alt_shift_lowercase("alt+shift+control+end", "ControlAltShiftEnd")] + #[case::modifier_control_alt_shift_mixedcase("aLt+shIft+conTrol+eNd", "ControlAltShiftEnd")] + #[case::modifier_control_alt_shift_out_of_order_1("Alt+Shift+Control+End", "ControlAltShiftEnd")] + #[case::modifier_control_alt_shift_out_of_order_2("Shift+Control+Alt+End", "ControlAltShiftEnd")] #[case::modifier_only_shift("Shift+End", "ShiftEnd")] #[case::modifier_only_control("Control+End", "ControlEnd")] - #[case::multiple("a b c d", "a,b,c,d")] + #[case::shift_with_printable_lower("Shift+a", "A")] + #[case::shift_with_printable_upper("Shift+A", "A")] + #[case::multiple("a b ẞ c d", "a,b,ẞ,c,d")] #[case::multiple_with_modifiers("Control+End Control+A", "ControlEnd,ControlA")] fn read_value(#[case] binding: &str, #[case] expected: &str) { with_git_config(&["[test]", format!("value = {binding}").as_str()], |git_config| { diff --git a/src/core/src/components/confirm/mod.rs b/src/core/src/components/confirm/mod.rs index dd76c0a..6c0d009 100644 --- a/src/core/src/components/confirm/mod.rs +++ b/src/core/src/components/confirm/mod.rs @@ -41,7 +41,6 @@ impl Confirm { if let Event::Key(key) = event { if let KeyCode::Char(c) = key.code { let mut event_lower_modifiers = key.modifiers; - event_lower_modifiers.remove(KeyModifiers::SHIFT); let event_lower = Event::Key(KeyEvent::new( KeyCode::Char(c.to_ascii_lowercase()), event_lower_modifiers, diff --git a/src/core/src/components/shared/editable_line.rs b/src/core/src/components/shared/editable_line.rs index 42e46e3..cb3167d 100644 --- a/src/core/src/components/shared/editable_line.rs +++ b/src/core/src/components/shared/editable_line.rs @@ -186,7 +186,7 @@ impl EditableLine { }, Event::Key(KeyEvent { code: KeyCode::Char(c), - modifiers: KeyModifiers::NONE | KeyModifiers::SHIFT, + modifiers: KeyModifiers::NONE, }) => { let start = UnicodeSegmentation::graphemes(self.content.as_str(), true) .take(self.cursor_position) @@ -485,10 +485,7 @@ mod tests { fn add_character_uppercase() { let mut editable_line = EditableLine::new(); editable_line.set_content("abcd"); - _ = editable_line.handle_event(Event::Key(KeyEvent { - code: KeyCode::Char('X'), - modifiers: KeyModifiers::SHIFT, - })); + _ = editable_line.handle_event(Event::from(KeyCode::Char('X'))); assert_rendered_output!( Options AssertRenderOptions::INCLUDE_TRAILING_WHITESPACE, view_data_from_editable_line!(&editable_line), diff --git a/src/input/src/event.rs b/src/input/src/event.rs index 9b57e10..7ad9d3d 100644 --- a/src/input/src/event.rs +++ b/src/input/src/event.rs @@ -93,13 +93,10 @@ mod tests { fn from_crossterm_key_event_char_with_modifier() { let event = Event::from(ct_event::Event::Key(ct_event::KeyEvent::new( KeyCode::Char('?'), - KeyModifiers::SHIFT, + KeyModifiers::ALT, ))); - assert_eq!( - event, - Event::Key(KeyEvent::new(KeyCode::Char('?'), KeyModifiers::SHIFT)) - ); + assert_eq!(event, Event::Key(KeyEvent::new(KeyCode::Char('?'), KeyModifiers::ALT))); } #[test] diff --git a/src/input/src/key_bindings.rs b/src/input/src/key_bindings.rs index ab22276..0c5184a 100644 --- a/src/input/src/key_bindings.rs +++ b/src/input/src/key_bindings.rs @@ -84,10 +84,7 @@ pub fn map_keybindings<CustomEvent: crate::CustomEvent>(bindings: &[String]) -> let key_number = k[1..].parse::<u8>().unwrap_or(1); KeyCode::F(key_number) }, - k => { - // printable characters cannot use shift - KeyCode::Char(k.chars().next().expect("Expected only one character from Char KeyCode")) - }, + k => KeyCode::Char(k.chars().next().expect("Expected only one character from Char KeyCode")), }; Event::Key(KeyEvent::new(code, modifiers)) }) @@ -168,11 +165,4 @@ mod tests { Event::from(key_code) ]); } - - #[test] - fn map_keybindings_key_code_char_remove_shift() { - assert_eq!(map_keybindings::<TestEvent>(&[String::from("ShiftA")]), vec![ - Event::from(KeyCode::Char('A')) - ]); - } } diff --git a/src/input/src/key_event.rs b/src/input/src/key_event.rs index 40d9a2a..2eaf13e 100644 --- a/src/input/src/key_event.rs +++ b/src/input/src/key_event.rs @@ -15,13 +15,11 @@ impl KeyEvent { #[must_use] #[inline] pub fn new(mut code: KeyCode, mut modifiers: KeyModifiers) -> Self { - // ensure that uppercase characters always have SHIFT + // normalize keys with the SHIFT modifier if let KeyCode::Char(c) = code { - if c.is_ascii_uppercase() { - modifiers.insert(KeyModifiers::SHIFT); - } - else if modifiers.contains(KeyModifiers::SHIFT) { + if modifiers.contains(KeyModifiers::SHIFT) { code = KeyCode::Char(c.to_ascii_uppercase()); + modifiers.remove(KeyModifiers::SHIFT); } } Self { code, modifiers } @@ -41,3 +39,67 @@ impl From<KeyCode> for KeyEvent { Self::new(code, KeyModifiers::empty()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn new_non_character() { + assert_eq!(KeyEvent::new(KeyCode::Backspace, KeyModifiers::ALT), KeyEvent { + code: KeyCode::Backspace, + modifiers: KeyModifiers::ALT + }); + } + + #[test] + fn new_lowercase_character_without_shift() { + assert_eq!(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE), KeyEvent { + code: KeyCode::Char('a'), + modifiers: KeyModifiers::NONE + }); + } + + #[test] + fn new_uppercase_character_without_shift() { + assert_eq!(KeyEvent::new(KeyCode::Char('A'), KeyModifiers::NONE), KeyEvent { + code: KeyCode::Char('A'), + modifiers: KeyModifiers::NONE + }); + } + + #[test] + fn new_lowercase_character_with_shift() { + assert_eq!(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::SHIFT), KeyEvent { + code: KeyCode::Char('A'), + modifiers: KeyModifiers::NONE + }); + } + + #[test] + fn new_uppercase_character_with_shift() { + assert_eq!(KeyEvent::new(KeyCode::Char('A'), KeyModifiers::SHIFT), KeyEvent { + code: KeyCode::Char('A'), + modifiers: KeyModifiers::NONE + }); + } + + #[test] + fn from_crossterm_key_event() { + assert_eq!( + KeyEvent::from(crossterm::event::KeyEvent::new(KeyCode::Char('a'), KeyModifiers::ALT)), + KeyEvent { + code: KeyCode::Char('a'), + modifiers: KeyModifiers::ALT + } + ); + } + + #[test] + fn from_keycode() { + assert_eq!(KeyEvent::from(KeyCode::Char('a')), KeyEvent { + code: KeyCode::Char('a'), + modifiers: KeyModifiers::NONE + }); + } +} |