diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 46c41f581..13f7ab12c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -34,6 +34,8 @@ New or improved bindings This feature depends on the terminal advertising via XTGETTCAP support for the ``indn`` and ``cuu`` terminfo capabilities, and on the terminal supporting Synchronized Output (which is used by fish to detect features). If any is missing, the binding falls back to ``clear-screen``. +- Bindings using shift with non-ASCII letters (such as :kbd:`ctrl-shift-ä`) are now supported. + If there is any modifier other than shift, this is the recommended notation (as opposed to :kbd:`ctrl-Ä`). Completions ^^^^^^^^^^^ diff --git a/share/functions/fish_default_key_bindings.fish b/share/functions/fish_default_key_bindings.fish index 70e2b4b6d..2ac035ff8 100644 --- a/share/functions/fish_default_key_bindings.fish +++ b/share/functions/fish_default_key_bindings.fish @@ -44,7 +44,7 @@ function fish_default_key_bindings -d "emacs-like key binds" bind --preset $argv ctrl-/ undo bind --preset $argv ctrl-_ undo # XTerm idiosyncracy, can get rid of this once we go full CSI u bind --preset $argv ctrl-z undo - bind --preset $argv ctrl-Z redo + bind --preset $argv ctrl-shift-z redo bind --preset $argv alt-/ redo bind --preset $argv alt-t transpose-words bind --preset $argv alt-u upcase-word diff --git a/src/builtins/fish_key_reader.rs b/src/builtins/fish_key_reader.rs index 2cb5ec98e..7b55185a6 100644 --- a/src/builtins/fish_key_reader.rs +++ b/src/builtins/fish_key_reader.rs @@ -22,7 +22,7 @@ terminal_protocols_enable_ifn, Capability, CharEvent, ImplicitEvent, InputEventQueue, InputEventQueuer, KeyEvent, KITTY_KEYBOARD_SUPPORTED, }, - key::{char_to_symbol, Key}, + key::{char_to_symbol, Key, Modifiers}, nix::isatty, panic::panic_handler, print_help::print_help, @@ -102,9 +102,33 @@ fn process_input(streams: &mut IoStreams, continuous_mode: bool, verbose: bool) } streams.out.append(L!("\n")); } - streams - .out - .append(sprintf!("bind %s 'do something'\n", kevt.key)); + let mut print_bind_example = |key: &Key, recommended: bool| { + streams.out.append(sprintf!( + "bind %s 'do something'%s\n", + key, + if recommended { + " # recommended notation" + } else { + "" + } + )); + }; + let have_shifted_key = kevt.key.shifted_codepoint != '\0'; + // If we have shift + some other modifier, the lowercase version is the canonical one. + let prefer_explicit_shift = kevt.key.modifiers.shift + && kevt.key.modifiers != Modifiers::SHIFT + && kevt + .key + .shifted_codepoint + .to_lowercase() + .eq(Some(kevt.key.codepoint).into_iter()); + if have_shifted_key { + let mut shifted_key = kevt.key.key; + shifted_key.modifiers.shift = false; + shifted_key.codepoint = kevt.key.shifted_codepoint; + print_bind_example(&shifted_key, !prefer_explicit_shift); + } + print_bind_example(&kevt.key, have_shifted_key && prefer_explicit_shift); if continuous_mode && should_exit(streams, &mut recent_chars, kevt.key) { streams.err.appendln("\nExiting at your request."); diff --git a/src/input_common.rs b/src/input_common.rs index 87913df57..dab9d27b9 100644 --- a/src/input_common.rs +++ b/src/input_common.rs @@ -145,12 +145,23 @@ pub enum ReadlineCmd { #[derive(Clone, Copy, Debug)] pub struct KeyEvent { pub key: Key, + pub shifted_codepoint: char, } impl KeyEvent { pub(crate) fn new(modifiers: Modifiers, codepoint: char) -> Self { Self::from(Key::new(modifiers, codepoint)) } + pub(crate) fn with_shifted_codepoint( + modifiers: Modifiers, + codepoint: char, + shifted_codepoint: Option, + ) -> Self { + Self { + key: Key::new(modifiers, codepoint), + shifted_codepoint: shifted_codepoint.unwrap_or_default(), + } + } pub(crate) fn from_raw(codepoint: char) -> Self { Self::from(Key::from_raw(codepoint)) } @@ -161,7 +172,10 @@ pub fn from_single_byte(c: u8) -> Self { impl From for KeyEvent { fn from(key: Key) -> Self { - Self { key } + Self { + key, + shifted_codepoint: '\0', + } } } @@ -178,12 +192,89 @@ fn deref_mut(&mut self) -> &mut Self::Target { } } +fn apply_shift(mut key: Key, do_ascii: bool, shifted_codepoint: char) -> Option { + if !key.modifiers.shift { + return Some(key); + } + if shifted_codepoint != '\0' { + key.codepoint = shifted_codepoint; + } else if do_ascii && key.codepoint.is_ascii_lowercase() { + // For backwards compatibility, we convert the "bind shift-a" notation to "bind A". + // This enables us to match "A" events which are the legacy encoding for keys that + // generate text -- until we request kitty's "Report all keys as escape codes". + // We do not currently convert non-ASCII key notation such as "bind shift-ä". + key.codepoint = key.codepoint.to_ascii_uppercase(); + } else { + return None; + }; + key.modifiers.shift = false; + Some(key) +} + impl PartialEq for KeyEvent { fn eq(&self, key: &Key) -> bool { - &self.key == key + if &self.key == key { + return true; + } + + let Some(shifted_evt) = apply_shift(self.key, false, self.shifted_codepoint) else { + return false; + }; + let Some(shifted_key) = apply_shift(*key, true, '\0') else { + return false; + }; + shifted_evt == shifted_key } } +#[test] +fn test_key_event_eq() { + let none = Modifiers::default(); + let shift = Modifiers::SHIFT; + let ctrl = Modifiers::CTRL; + let ctrl_shift = Modifiers { + ctrl: true, + shift: true, + ..Default::default() + }; + + assert_eq!(KeyEvent::new(none, 'a'), Key::new(none, 'a')); + assert_ne!(KeyEvent::new(none, 'a'), Key::new(none, 'A')); + assert_eq!(KeyEvent::new(shift, 'a'), Key::new(shift, 'a')); + assert_ne!(KeyEvent::new(shift, 'a'), Key::new(none, 'A')); + assert_ne!(KeyEvent::new(shift, 'ä'), Key::new(none, 'Ä')); + // For historical reasons we canonicalize notation for ASCII keys like "shift-a" to "A", + // but not "shift-a" events - those should send a shifted key. + assert_eq!(KeyEvent::new(none, 'A'), Key::new(shift, 'a')); + assert_ne!(KeyEvent::new(none, 'A'), Key::new(shift, 'A')); + assert_eq!(KeyEvent::new(none, 'Ä'), Key::new(none, 'Ä')); + assert_ne!(KeyEvent::new(none, 'Ä'), Key::new(shift, 'ä')); + + // FYI: for codepoints that are not letters with uppercase/lowercase versions, we use + // the shifted key in the canonical notation, because the unshifted one may depend on the + // keyboard layout. + let ctrl_shift_equals = KeyEvent::with_shifted_codepoint(ctrl_shift, '=', Some('+')); + assert_eq!(ctrl_shift_equals, Key::new(ctrl_shift, '=')); + assert_eq!(ctrl_shift_equals, Key::new(ctrl, '+')); // canonical notation + assert_ne!(ctrl_shift_equals, Key::new(ctrl_shift, '+')); + assert_ne!(ctrl_shift_equals, Key::new(ctrl, '=')); + + // A event like capslock-shift-ä may or may not include a shifted codepoint. + // + // Without a shifted codepoint, we cannot easily match ctrl-Ä. + let caps_ctrl_shift_ä = KeyEvent::new(ctrl_shift, 'ä'); + assert_eq!(caps_ctrl_shift_ä, Key::new(ctrl_shift, 'ä')); // canonical notation + assert_ne!(caps_ctrl_shift_ä, Key::new(ctrl, 'ä')); + assert_ne!(caps_ctrl_shift_ä, Key::new(ctrl, 'Ä')); // can't match without shifted key + assert_ne!(caps_ctrl_shift_ä, Key::new(ctrl_shift, 'Ä')); + // With a shifted codepoint, we can match the alternative notation too. + let caps_ctrl_shift_ä = KeyEvent::with_shifted_codepoint(ctrl_shift, 'ä', Some('Ä')); + assert_eq!(caps_ctrl_shift_ä, Key::new(ctrl_shift, 'ä')); // canonical notation + assert_ne!(caps_ctrl_shift_ä, Key::new(ctrl, 'ä')); + assert_eq!(caps_ctrl_shift_ä, Key::new(ctrl, 'Ä')); // matched via shifted key + assert_ne!(caps_ctrl_shift_ä, Key::new(ctrl_shift, 'Ä')); +} + /// Represents an event on the character input stream. #[derive(Debug, Clone)] pub enum CharEventType { @@ -620,13 +711,15 @@ pub(crate) fn terminal_protocols_disable_ifn() { did_write.store(true); } -fn parse_mask(mask: u32) -> Modifiers { - Modifiers { +fn parse_mask(mask: u32) -> (Modifiers, bool) { + let modifiers = Modifiers { ctrl: (mask & 4) != 0, alt: (mask & 2) != 0, shift: (mask & 1) != 0, sup: (mask & 8) != 0, - } + }; + let caps_lock = (mask & 64) != 0; + (modifiers, caps_lock) } // A data type used by the input machinery. @@ -1027,16 +1120,35 @@ fn parse_csi(&mut self, buffer: &mut Vec) -> Option { return None; } - let masked_key = |mut codepoint, shifted_codepoint| { + let masked_key = |codepoint: char, shifted_codepoint: Option| { let mask = params[1][0].saturating_sub(1); - let mut modifiers = parse_mask(mask); - if let Some(shifted_codepoint) = shifted_codepoint { - if shifted_codepoint != '\0' && modifiers.shift { - modifiers.shift = false; - codepoint = shifted_codepoint; - } + let (mut modifiers, caps_lock) = parse_mask(mask); + + // An event like "capslock-shift-=" should have a shifted codepoint ("+") to enable + // fish to match "bind +". + // + // With letters that are affected by capslock, capslock and shift cancel each + // other out ("capslock-shift-ä"), unless there is another modifier to imply that + // capslock should be ignored. + // + // So if shift is the only modifier, we should consume it, but not if the event is + // something like "capslock-shift-delete" because delete is not affected by capslock. + // + // Normally, we could consume shift by translating to the shifted key. + // While capslock is on however, we don't get a shifted key, see + // https://github.com/kovidgoyal/kitty/issues/8493. + // + // Do it by trying to find out ourselves whether the key is affected by capslock. + // + // Alternatively, we could relax our exact matching semantics, and make "bind ä" + // match the "shift-ä" event, as suggested in the kitty issue. + if caps_lock + && modifiers == Modifiers::SHIFT + && !codepoint.to_uppercase().eq(Some(codepoint).into_iter()) + { + modifiers.shift = false; } - KeyEvent::new(modifiers, codepoint) + KeyEvent::with_shifted_codepoint(modifiers, codepoint, shifted_codepoint) }; let key = match c { @@ -1098,7 +1210,7 @@ fn parse_csi(&mut self, buffer: &mut Vec) -> Option { return invalid_sequence(buffer); }; let position = ViewportPosition { x, y }; - let modifiers = parse_mask((button >> 2) & 0x07); + let (modifiers, _caps_lock) = parse_mask((button >> 2) & 0x07); let code = button & 0x43; if code != 0 || c != b'M' || modifiers.is_some() { return None; @@ -1312,7 +1424,7 @@ fn parse_ss3(&mut self, buffer: &mut Vec) -> Option { raw_mask = raw_mask * 10 + u32::from(code - b'0'); code = self.try_readb(buffer).unwrap_or(0xff); } - let modifiers = parse_mask(raw_mask.saturating_sub(1)); + let (modifiers, _caps_lock) = parse_mask(raw_mask.saturating_sub(1)); #[rustfmt::skip] let key = match code { b' ' => KeyEvent::new(modifiers, key::Space), diff --git a/src/key.rs b/src/key.rs index c15d9e721..9ce385de7 100644 --- a/src/key.rs +++ b/src/key.rs @@ -72,11 +72,22 @@ const fn new() -> Self { sup: false, } } + #[cfg(test)] + pub(crate) const CTRL: Self = { + let mut m = Self::new(); + m.ctrl = true; + m + }; pub(crate) const ALT: Self = { let mut m = Self::new(); m.alt = true; m }; + pub(crate) const SHIFT: Self = { + let mut m = Self::new(); + m.shift = true; + m + }; pub(crate) fn is_some(&self) -> bool { *self != Self::new() } @@ -212,19 +223,6 @@ pub(crate) fn canonicalize_key(mut key: Key) -> Result { key.modifiers.ctrl = true; } } - if key.modifiers.shift { - if key.codepoint.is_ascii_lowercase() { - // Shift + ASCII letters is just the uppercase letter. - key.modifiers.shift = false; - key.codepoint = key.codepoint.to_ascii_uppercase(); - } else if !fish_is_pua(key.codepoint) { - // Shift + any other printable character is not allowed. - return Err(wgettext_fmt!( - "Shift modifier is only supported on special keys and lowercase ASCII, not '%s'", - key, - )); - } - } Ok(key) } diff --git a/tests/checks/bind.fish b/tests/checks/bind.fish index ed2c3662f..5d199eb5f 100644 --- a/tests/checks/bind.fish +++ b/tests/checks/bind.fish @@ -163,4 +163,19 @@ bind \n 2>&1 bind _\cx_\ci_\ei_\\_\'_ 'echo foo' # CHECKERR: bind: cannot parse key '_\cx_\t_\ei_\\_'_' +bind A +# CHECKERR: bind: No binding found for key 'A' + +bind shift-a +# CHECKERR: bind: No binding found for key 'shift-a' + +bind shift-A +# CHECKERR: bind: No binding found for key 'shift-A' + +bind ctrl-shift-a +# CHECKERR: bind: No binding found for key 'ctrl-shift-a' + +bind ctrl-shift-ä +# CHECKERR: bind: No binding found for key 'ctrl-shift-ä' + exit 0