From 6f6ae349759f1432d76ced42df3c75ce93cbecb2 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sun, 5 Jan 2025 08:35:13 +0100 Subject: [PATCH] Disable helpful macOS remappings if kitty disambiguate flag is set (Originally reported at https://github.com/fish-shell/fish-shell/issues/11004) Commit 5bb2c62f (config: revert cmd/alt+left/right to legacy encoding on macOS by default, 2024-12-24) translates keys like alt-left to alt-b on macOS. That behavior is part of the default Terminal.app and allows alt-left to do something reasonable on the default macOS shell (zsh). Other applications want to disambiguate alt-left from alt-b. Ror example, fish maps them to different commands. fish requests the kitty keyboard protocol, specifically the [disambiguate flag (0b1)](https://sw.kovidgoyal.net/kitty/keyboard-protocol/#disambiguate-escape-codes). I don't know if the protocol adjudicates this specific question but I think the disambiguate flag is a good signal that helpful remappings are not desired. Let's ignore these bindings while disambiguate is set. In future we could try to find a better criteria. To test, it should be enough to run (in bash or zsh) printf '\x1b[=1u' and check if the keys change accordingly. Unfortunately I can't get XCode to run on my Mac right now, but I was able to test the change by applying the "Natural text editing" keybinds on Linux. Here's me typing the affected keys into "ghostty -e fish_key_reader -V", before and after. Looks good, except that fish does not support the super modifier yet. super-left # decoded from: \x01 bind ctrl-a 'do something' # decoded from: \e\[1\;9D bind left 'do something' super-right # decoded from: \x05 bind ctrl-e 'do something' # decoded from: \e\[1\;9C bind right 'do something' super-backspace # decoded from: \e\x15 bind ctrl-alt-u 'do something' # decoded from: \e\[127\;9u bind backspace 'do something' alt-left # decoded from: \eb bind alt-b 'do something' # decoded from: \e\[1\;3D bind alt-left 'do something' alt-right # decoded from: \ef bind alt-f 'do something' # decoded from: \e\[1\;3C bind alt-right 'do something' --- src/Surface.zig | 26 +++++++++++++++++++++++--- src/apprt/embedded.zig | 4 ++++ src/config/Config.zig | 10 +++++----- src/input/Binding.zig | 21 +++++++++++---------- 4 files changed, 43 insertions(+), 18 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index 1dc10fb27..9485d4b9d 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -1833,6 +1833,22 @@ pub fn keyCallback( return .consumed; } +pub fn isBindingMaskedByKittyDisambiguate( + self: *Surface, + action: input.Binding.Action, +) bool { + switch (action) { + .esc_unless_kitty_disambiguate, + .text_unless_kitty_disambiguate => { + if (self.io.terminal.screen.kitty_keyboard.current().disambiguate) { + return true; + } + }, + else => {}, + } + return false; +} + /// Maybe handles a binding for a given event and if so returns the effect. /// Returns null if the event is not handled in any way and processing should /// continue. @@ -1916,6 +1932,10 @@ fn maybeHandleBinding( }; const action = leaf.action; + if (self.isBindingMaskedByKittyDisambiguate(action)) { + return null; + } + // consumed determines if the input is consumed or if we continue // encoding the key (if we have a key to encode). const consumed = consumed: { @@ -3826,14 +3846,14 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !bool } switch (action.scoped(.surface).?) { - .csi, .esc => |data| { + .csi, .esc_unless_kitty_disambiguate => |data| { // We need to send the CSI/ESC sequence as a single write request. // If you split it across two then the shell can interpret it // as two literals. var buf: [128]u8 = undefined; const full_data = switch (action) { .csi => try std.fmt.bufPrint(&buf, "\x1b[{s}", .{data}), - .esc => try std.fmt.bufPrint(&buf, "\x1b{s}", .{data}), + .esc_unless_kitty_disambiguate => try std.fmt.bufPrint(&buf, "\x1b{s}", .{data}), else => unreachable, }; self.io.queueMessage(try termio.Message.writeReq( @@ -3851,7 +3871,7 @@ pub fn performBindingAction(self: *Surface, action: input.Binding.Action) !bool } }, - .text => |data| { + .text_unless_kitty_disambiguate => |data| { // For text we always allocate just because its easier to // handle all cases that way. const buf = try self.alloc.alloc(u8, data.len); diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 10d09988d..fabd6fab1 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -1825,6 +1825,10 @@ pub const CAPI = struct { return false; }; + if (ptr.isBindingMaskedByKittyDisambiguate(action)) { + return false; + } + _ = ptr.core_surface.performBindingAction(action) catch |err| { log.err("error performing binding action action={} err={}", .{ action, err }); return false; diff --git a/src/config/Config.zig b/src/config/Config.zig index cbeafdd1c..40bd314b7 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -2715,27 +2715,27 @@ pub fn default(alloc_gpa: Allocator) Allocator.Error!Config { try result.keybind.set.put( alloc, .{ .key = .{ .translated = .right }, .mods = .{ .super = true } }, - .{ .text = "\\x05" }, + .{ .text_unless_kitty_disambiguate = "\\x05" }, ); try result.keybind.set.put( alloc, .{ .key = .{ .translated = .left }, .mods = .{ .super = true } }, - .{ .text = "\\x01" }, + .{ .text_unless_kitty_disambiguate = "\\x01" }, ); try result.keybind.set.put( alloc, .{ .key = .{ .translated = .backspace }, .mods = .{ .super = true } }, - .{ .esc = "\x15" }, + .{ .esc_unless_kitty_disambiguate = "\x15" }, ); try result.keybind.set.put( alloc, .{ .key = .{ .translated = .left }, .mods = .{ .alt = true } }, - .{ .esc = "b" }, + .{ .esc_unless_kitty_disambiguate = "b" }, ); try result.keybind.set.put( alloc, .{ .key = .{ .translated = .right }, .mods = .{ .alt = true } }, - .{ .esc = "f" }, + .{ .esc_unless_kitty_disambiguate = "f" }, ); } diff --git a/src/input/Binding.zig b/src/input/Binding.zig index e0ad6c571..3ecff992a 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -233,13 +233,14 @@ pub const Action = union(enum) { /// CSI header (`ESC [` or `\x1b[`). csi: []const u8, - /// Send an `ESC` sequence. - esc: []const u8, + /// Unless kitty disambiguate is active, send an `ESC` sequence. + esc_unless_kitty_disambiguate: []const u8, - // Send the given text. Uses Zig string literal syntax. This is currently - // not validated. If the text is invalid (i.e. contains an invalid escape - // sequence), the error will currently only show up in logs. - text: []const u8, + // Unless kitty disambiguate is active, send the given text. Uses Zig + // string literal syntax. This is currently not validated. If the text + // is invalid (i.e. contains an invalid escape sequence), the error will + // currently only show up in logs. + text_unless_kitty_disambiguate: []const u8, /// Send data to the pty depending on whether cursor key mode is enabled /// (`application`) or disabled (`normal`). @@ -702,8 +703,8 @@ pub const Action = union(enum) { // Obviously surface actions. .csi, - .esc, - .text, + .esc_unless_kitty_disambiguate, + .text_unless_kitty_disambiguate, .cursor_key, .reset, .copy_to_clipboard, @@ -1897,8 +1898,8 @@ test "parse: action with string" { // parameter { const binding = try parseSingle("a=esc:A"); - try testing.expect(binding.action == .esc); - try testing.expectEqualStrings("A", binding.action.esc); + try testing.expect(binding.action == .esc_unless_kitty_disambiguate); + try testing.expectEqualStrings("A", binding.action.esc_unless_kitty_disambiguate); } }