mirror of
https://github.com/fish-shell/fish-shell.git
synced 2026-05-25 06:41:15 -03:00
Allow explicit shift modifier for non-ASCII letters, fix capslock behavior
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:
@@ -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
|
||||
^^^^^^^^^^^
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.");
|
||||
|
||||
@@ -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),
|
||||
|
||||
24
src/key.rs
24
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, WString> {
|
||||
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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user