h/fish-shell
1
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2026-05-20 18:41:16 -03:00
Code Issues Packages Projects Releases Wiki Activity

Allow explicit shift modifier for non-ASCII letters, fix capslock behavior

Browse Source 
We canonicalize "ctrl-shift-i" to "ctrl-I".
Both when deciphering this notation (as given to builtin bind),
and when receiving it as a key event ("\e[105;73;6u")

This has problems:

A. Our bind notation canonicalization only works for 26 English letters.
   For example, "ctrl-shift-ä" is not supported -- only "ctrl-Ä" is.
   We could try to fix that but this depends on the keyboard layout.
   For example "bind alt-shift-=" and "bind alt-+" are equivalent on a "us"
   layout but not on a "de" layout.
B. While capslock is on, the key event won't include a shifted key ("73" here).
   This is due a quirk in the kitty keyboard protocol[^1].  This means that
   fish_key_reader's canonicalization doesn't work (unless we call toupper()
   ourselves).

I think we want to support both notations.

It's recommended to match all of these (in this order) when pressing
"ctrl-shift-i".

	1. bind ctrl-shift-i do-something
	2. bind ctrl-shift-I do-something
	3. bind ctrl-I do-something
	4. bind ctrl-i do-something

Support 1 and 3 for now, allowing both bindings to coexist. No priorities
for now. This solves problem A, and -- if we take care to use the explicit
shift notation -- problem B.

For keys that are not affected by capslock, problem B does not apply.  In this
case, recommend the shifted notation ("alt-+" instead of "alt-shift-=")
since that seems more intuitive.
Though if we prioritized "alt-shift-=" over "alt-+" as per the recommendation,
that's an argument against the shifted key.

Example output for some key events:

	$ fish_key_reader -cV
	# decoded from: \e\[61:43\;4u
	bind alt-+ 'do something' # recommended notation
	bind alt-shift-= 'do something'
	# decoded from: \e\[61:43\;68u
	bind alt-+ 'do something' # recommended notation
	bind alt-shift-= 'do something'

	# decoded from: \e\[105:73\;6u
	bind ctrl-I 'do something'
	bind ctrl-shift-i 'do something' # recommended notation
	# decoded from: \e\[105\;70u
	bind ctrl-shift-i 'do something'

Due to the capslock quirk, the last one has only one matching representation
since there is no shifted key.  We could decide to match ctrl-shift-i events
(that don't have a shifted key) to ctrl-I bindings (for ASCII letters), as
before this patch. But that case is very rare, it should only happen when
capslock is on, so it's probably not even a breaking change.

The other way round is supported -- we do match ctrl-I events (typically
with shifted key) to ctrl-shift-i bindings (but only for ASCII letters).
This is mainly for backwards compatibility.

Also note that, bindings without other modifiers currently need to use the
shifted key (like "Ä", not "shift-ä"), since we still get a legacy encoding,
until we request "Report all keys as escape codes".

[^1]: <https://github.com/kovidgoyal/kitty/issues/8493>
This commit is contained in:
Johannes Altmanninger
2025-03-30 08:33:24 +02:00
parent 7f25d865a9
commit 50a6e486a5
6 changed files with 184 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

142
src/input_common.rs
View File

@@ -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<char>,
) -> 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<Key> 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<Key> {
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<Key> 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<u8>) -> Option<KeyEvent> {
return None;
}
let masked_key = |mut codepoint, shifted_codepoint| {
let masked_key = |codepoint: char, shifted_codepoint: Option<char>| {
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<u8>) -> Option<KeyEvent> {
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<u8>) -> Option<KeyEvent> {
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),