From 4f2d67d8f3239828e9370b24ff1400b565d6118d Mon Sep 17 00:00:00 2001 From: Tim Culverhouse Date: Thu, 28 Sep 2023 10:12:43 -0500 Subject: [PATCH 1/3] gtk(input): fix value used for lowercase lower_unicode When converting keyval to the unshifted version, gdk_keyval_to_lower returns a keyval type, not a unicode value. Convert this to unicode before assigning to keyval_unicode_unshifted. This fixes a bug where the incorrect keycode is sent in alternate layouts with kitty keyboard on linux. --- src/apprt/gtk/Surface.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/apprt/gtk/Surface.zig b/src/apprt/gtk/Surface.zig index 09aa6206a..50a88401e 100644 --- a/src/apprt/gtk/Surface.zig +++ b/src/apprt/gtk/Surface.zig @@ -757,8 +757,8 @@ fn keyEvent( // Norwegian, and French layouts and thats what we have real users for // right now. const lower = c.gdk_keyval_to_lower(keyval); - if (std.math.cast(u21, lower)) |val| break :unshifted val; - break :unshifted 0; + const lower_unicode = c.gdk_keyval_to_unicode(lower); + break :unshifted std.math.cast(u21, lower_unicode) orelse 0; }; // We always reset our committed text when ending a keypress so that From fb649e689dd7f7007061fc30ac1b3e80f1718d97 Mon Sep 17 00:00:00 2001 From: Tim Culverhouse Date: Thu, 28 Sep 2023 12:23:20 -0500 Subject: [PATCH 2/3] input(kitty): fix reporting of alternate keys Fix reporting of alternate keys when using the kitty protocol. Alternate keyboard layouts were failing to report the "base layout" key. This implementation now matches kitty's output 1:1, and has some added unit tests for cyrillic characters. This also fixes a bug where a caps_lock modified key would report the shifted key as well. The protocol explicitly requires that shifted keys are only reported if the shift modifier is true. --- src/input/KeyEncoder.zig | 149 ++++++++++++++++++++++++++++++++++++--- src/input/key.zig | 72 +++++++++++++++++++ 2 files changed, 211 insertions(+), 10 deletions(-) diff --git a/src/input/KeyEncoder.zig b/src/input/KeyEncoder.zig index 85b39baca..1fe44425b 100644 --- a/src/input/KeyEncoder.zig +++ b/src/input/KeyEncoder.zig @@ -143,12 +143,25 @@ fn kitty( } if (self.kitty_flags.report_alternates) alternates: { - const view = try std.unicode.Utf8View.init(self.event.utf8); - var it = view.iterator(); - const cp = it.nextCodepoint() orelse break :alternates; - if (it.nextCodepoint() != null) break :alternates; - if (cp != seq.key and !isControl(cp)) { - seq.alternates = &.{cp}; + // Break early if this is a control key + if (isControl(seq.key)) break :alternates; + + // Set the first alternate (shifted version) + { + const view = try std.unicode.Utf8View.init(self.event.utf8); + var it = view.iterator(); + // We break early if there are codepoints...there are no + // alternate key(s) to report + const shifted = it.nextCodepoint() orelse break :alternates; + // Only report the shifted key if we have a shift modifier + if (shifted != seq.key and seq.mods.shift) seq.alternates[0] = shifted; + } + + // Set the base layout key + { + if (self.event.key.codepoint()) |base| { + if (base != seq.key) seq.alternates[1] = base; + } } } @@ -549,7 +562,7 @@ const KittySequence = struct { final: u8, mods: KittyMods = .{}, event: Event = .none, - alternates: []const u21 = &.{}, + alternates: [2]?u21 = .{ null, null }, text: []const u8 = "", /// Values for the event code (see "event-type" in above comment). @@ -577,7 +590,17 @@ const KittySequence = struct { // Key section try writer.print("\x1B[{d}", .{self.key}); - for (self.alternates) |alt| try writer.print(":{d}", .{alt}); + // Write our alternates + if (self.alternates[0]) |shifted| { + try writer.print(":{d}", .{shifted}); + } + if (self.alternates[1]) |base| { + if (self.alternates[0] == null) { + try writer.print("::{d}", .{base}); + } else { + try writer.print(":{d}", .{base}); + } + } // Mods and events section const mods = self.mods.seqInt(); @@ -897,9 +920,115 @@ test "kitty: matching unshifted codepoint" { // WARNING: This is not a valid encoding. This is a hypothetical encoding // just to test that our logic is correct around matching unshifted - // codepoints. + // codepoints. We get an alternate here because the unshifted_codepoint does + // not match the base key const actual = try enc.kitty(&buf); - try testing.expectEqualStrings("\x1b[65;2u", actual); + try testing.expectEqualStrings("\x1b[65::97;2u", actual); +} + +test "kitty: report alternates with caps" { + var buf: [128]u8 = undefined; + var enc: KeyEncoder = .{ + .event = .{ + .key = .j, + .mods = .{ .caps_lock = true }, + .utf8 = "J", + .unshifted_codepoint = 106, + }, + .kitty_flags = .{ + .disambiguate = true, + .report_all = true, + .report_alternates = true, + .report_associated = true, + }, + }; + + const actual = try enc.kitty(&buf); + try testing.expectEqualStrings("\x1b[106;65;74u", actual); +} + +test "kitty: report alternates colon (shift+';')" { + var buf: [128]u8 = undefined; + var enc: KeyEncoder = .{ + .event = .{ + .key = .semicolon, + .mods = .{ .shift = true }, + .utf8 = ":", + .unshifted_codepoint = ';', + }, + .kitty_flags = .{ + .disambiguate = true, + .report_all = true, + .report_alternates = true, + .report_associated = true, + }, + }; + + const actual = try enc.kitty(&buf); + try testing.expectEqualStrings("\x1b[59:58;2;58u", actual); +} + +test "kitty: report alternates with ru layout" { + var buf: [128]u8 = undefined; + var enc: KeyEncoder = .{ + .event = .{ + .key = .semicolon, + .mods = .{}, + .utf8 = "ч", + .unshifted_codepoint = 1095, + }, + .kitty_flags = .{ + .disambiguate = true, + .report_all = true, + .report_alternates = true, + .report_associated = true, + }, + }; + + const actual = try enc.kitty(&buf); + try testing.expectEqualStrings("\x1b[1095::59;;1095u", actual); +} + +test "kitty: report alternates with ru layout shifted" { + var buf: [128]u8 = undefined; + var enc: KeyEncoder = .{ + .event = .{ + .key = .semicolon, + .mods = .{ .shift = true }, + .utf8 = "Ч", + .unshifted_codepoint = 1095, + }, + .kitty_flags = .{ + .disambiguate = true, + .report_all = true, + .report_alternates = true, + .report_associated = true, + }, + }; + + const actual = try enc.kitty(&buf); + try testing.expectEqualStrings("\x1b[1095:1063:59;2;1063u", actual); +} + +test "kitty: report alternates with ru layout caps lock" { + var buf: [128]u8 = undefined; + var enc: KeyEncoder = .{ + .event = .{ + .key = .semicolon, + .mods = .{ .caps_lock = true }, + .utf8 = "Ч", + .unshifted_codepoint = 1095, + }, + .kitty_flags = .{ + .disambiguate = true, + .report_all = true, + .report_alternates = true, + .report_associated = true, + }, + }; + + const actual = try enc.kitty(&buf); + try testing.expectEqualStrings("\x1b[1095::59;65;1063u", actual); } // macOS generates utf8 text for arrow keys. diff --git a/src/input/key.zig b/src/input/key.zig index 830501772..bb8f108bf 100644 --- a/src/input/key.zig +++ b/src/input/key.zig @@ -448,4 +448,76 @@ pub const Key = enum(c_int) { else => false, }; } + + // Returns the codepoint representing this key, or null if the key is not + // printable + pub fn codepoint(self: Key) ?u21 { + return switch (self) { + .a => 'a', + .b => 'b', + .c => 'c', + .d => 'd', + .e => 'e', + .f => 'f', + .g => 'g', + .h => 'h', + .i => 'i', + .j => 'j', + .k => 'k', + .l => 'l', + .m => 'm', + .n => 'n', + .o => 'o', + .p => 'p', + .q => 'q', + .r => 'r', + .s => 's', + .t => 't', + .u => 'u', + .v => 'v', + .w => 'w', + .x => 'x', + .y => 'y', + .z => 'z', + .zero => '0', + .one => '1', + .two => '2', + .three => '3', + .four => '4', + .five => '5', + .six => '6', + .seven => '7', + .eight => '8', + .nine => '9', + .semicolon => ';', + .space => ' ', + .apostrophe => '\'', + .comma => ',', + .grave_accent => '`', + .period => '.', + .slash => '/', + .minus => '-', + .equal => '=', + .left_bracket => '[', + .right_bracket => ']', + .backslash => '\\', + .kp_0 => '0', + .kp_1 => '1', + .kp_2 => '2', + .kp_3 => '3', + .kp_4 => '4', + .kp_5 => '5', + .kp_6 => '6', + .kp_7 => '7', + .kp_8 => '8', + .kp_9 => '9', + .kp_decimal => '.', + .kp_divide => '/', + .kp_multiply => '*', + .kp_subtract => '-', + .kp_add => '+', + .kp_equal => '=', + else => null, + }; + } }; From a2e2889f2be57146fd9bd00a01de988770163aff Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 29 Sep 2023 11:52:40 -0700 Subject: [PATCH 3/3] input: make Key ascii functions comptime-generated --- src/input/KeyEncoder.zig | 10 +- src/input/key.zig | 312 ++++++++++++++------------------------- 2 files changed, 115 insertions(+), 207 deletions(-) diff --git a/src/input/KeyEncoder.zig b/src/input/KeyEncoder.zig index 1fe44425b..fb38f5fc8 100644 --- a/src/input/KeyEncoder.zig +++ b/src/input/KeyEncoder.zig @@ -158,10 +158,8 @@ fn kitty( } // Set the base layout key - { - if (self.event.key.codepoint()) |base| { - if (base != seq.key) seq.alternates[1] = base; - } + if (self.event.key.codepoint()) |base| { + if (base != seq.key) seq.alternates[1] = base; } } @@ -591,9 +589,7 @@ const KittySequence = struct { // Key section try writer.print("\x1B[{d}", .{self.key}); // Write our alternates - if (self.alternates[0]) |shifted| { - try writer.print(":{d}", .{shifted}); - } + if (self.alternates[0]) |shifted| try writer.print(":{d}", .{shifted}); if (self.alternates[1]) |base| { if (self.alternates[0] == null) { try writer.print("::{d}", .{base}); diff --git a/src/input/key.zig b/src/input/key.zig index bb8f108bf..aabd3e2e3 100644 --- a/src/input/key.zig +++ b/src/input/key.zig @@ -298,154 +298,45 @@ pub const Key = enum(c_int) { /// are independent of the physical key. pub fn fromASCII(ch: u8) ?Key { return switch (ch) { - 'a' => .a, - 'b' => .b, - 'c' => .c, - 'd' => .d, - 'e' => .e, - 'f' => .f, - 'g' => .g, - 'h' => .h, - 'i' => .i, - 'j' => .j, - 'k' => .k, - 'l' => .l, - 'm' => .m, - 'n' => .n, - 'o' => .o, - 'p' => .p, - 'q' => .q, - 'r' => .r, - 's' => .s, - 't' => .t, - 'u' => .u, - 'v' => .v, - 'w' => .w, - 'x' => .x, - 'y' => .y, - 'z' => .z, - '0' => .zero, - '1' => .one, - '2' => .two, - '3' => .three, - '4' => .four, - '5' => .five, - '6' => .six, - '7' => .seven, - '8' => .eight, - '9' => .nine, - ';' => .semicolon, - ' ' => .space, - '\'' => .apostrophe, - ',' => .comma, - '`' => .grave_accent, - '.' => .period, - '/' => .slash, - '-' => .minus, - '=' => .equal, - '[' => .left_bracket, - ']' => .right_bracket, - '\\' => .backslash, - else => null, + inline else => |comptime_ch| { + return comptime result: { + @setEvalBranchQuota(100_000); + for (codepoint_map) |entry| { + if (entry[0] == @as(u21, @intCast(comptime_ch))) { + break :result entry[1]; + } + } + + break :result null; + }; + }, }; } /// True if this key represents a printable character. pub fn printable(self: Key) bool { return switch (self) { - .a, - .b, - .c, - .d, - .e, - .f, - .g, - .h, - .i, - .j, - .k, - .l, - .m, - .n, - .o, - .p, - .q, - .r, - .s, - .t, - .u, - .v, - .w, - .x, - .y, - .z, - .zero, - .one, - .two, - .three, - .four, - .five, - .six, - .seven, - .eight, - .nine, - .semicolon, - .space, - .apostrophe, - .comma, - .grave_accent, - .period, - .slash, - .minus, - .equal, - .left_bracket, - .right_bracket, - .backslash, - .kp_0, - .kp_1, - .kp_2, - .kp_3, - .kp_4, - .kp_5, - .kp_6, - .kp_7, - .kp_8, - .kp_9, - .kp_decimal, - .kp_divide, - .kp_multiply, - .kp_subtract, - .kp_add, - .kp_equal, - => true, + inline else => |tag| { + return comptime result: { + @setEvalBranchQuota(10_000); + for (codepoint_map) |entry| { + if (entry[1] == tag) break :result true; + } - else => false, + break :result false; + }; + }, }; } /// Returns true if this is a keypad key. pub fn keypad(self: Key) bool { return switch (self) { - .kp_0, - .kp_1, - .kp_2, - .kp_3, - .kp_4, - .kp_5, - .kp_6, - .kp_7, - .kp_8, - .kp_9, - .kp_decimal, - .kp_divide, - .kp_multiply, - .kp_subtract, - .kp_add, - .kp_enter, - .kp_equal, - => true, - - else => false, + inline else => |tag| { + const name = @tagName(tag); + const result = comptime std.mem.startsWith(u8, name, "kp_"); + return result; + }, }; } @@ -453,71 +344,92 @@ pub const Key = enum(c_int) { // printable pub fn codepoint(self: Key) ?u21 { return switch (self) { - .a => 'a', - .b => 'b', - .c => 'c', - .d => 'd', - .e => 'e', - .f => 'f', - .g => 'g', - .h => 'h', - .i => 'i', - .j => 'j', - .k => 'k', - .l => 'l', - .m => 'm', - .n => 'n', - .o => 'o', - .p => 'p', - .q => 'q', - .r => 'r', - .s => 's', - .t => 't', - .u => 'u', - .v => 'v', - .w => 'w', - .x => 'x', - .y => 'y', - .z => 'z', - .zero => '0', - .one => '1', - .two => '2', - .three => '3', - .four => '4', - .five => '5', - .six => '6', - .seven => '7', - .eight => '8', - .nine => '9', - .semicolon => ';', - .space => ' ', - .apostrophe => '\'', - .comma => ',', - .grave_accent => '`', - .period => '.', - .slash => '/', - .minus => '-', - .equal => '=', - .left_bracket => '[', - .right_bracket => ']', - .backslash => '\\', - .kp_0 => '0', - .kp_1 => '1', - .kp_2 => '2', - .kp_3 => '3', - .kp_4 => '4', - .kp_5 => '5', - .kp_6 => '6', - .kp_7 => '7', - .kp_8 => '8', - .kp_9 => '9', - .kp_decimal => '.', - .kp_divide => '/', - .kp_multiply => '*', - .kp_subtract => '-', - .kp_add => '+', - .kp_equal => '=', - else => null, + inline else => |tag| { + return comptime result: { + @setEvalBranchQuota(10_000); + for (codepoint_map) |entry| { + if (entry[1] == tag) break :result entry[0]; + } + + break :result null; + }; + }, }; } + + test "keypad keys" { + const testing = std.testing; + try testing.expect(Key.kp_0.keypad()); + try testing.expect(!Key.one.keypad()); + } + + const codepoint_map: []const struct { u21, Key } = &.{ + .{ 'a', .a }, + .{ 'b', .b }, + .{ 'c', .c }, + .{ 'd', .d }, + .{ 'e', .e }, + .{ 'f', .f }, + .{ 'g', .g }, + .{ 'h', .h }, + .{ 'i', .i }, + .{ 'j', .j }, + .{ 'k', .k }, + .{ 'l', .l }, + .{ 'm', .m }, + .{ 'n', .n }, + .{ 'o', .o }, + .{ 'p', .p }, + .{ 'q', .q }, + .{ 'r', .r }, + .{ 's', .s }, + .{ 't', .t }, + .{ 'u', .u }, + .{ 'v', .v }, + .{ 'w', .w }, + .{ 'x', .x }, + .{ 'y', .y }, + .{ 'z', .z }, + .{ '0', .zero }, + .{ '1', .one }, + .{ '2', .two }, + .{ '3', .three }, + .{ '4', .four }, + .{ '5', .five }, + .{ '6', .six }, + .{ '7', .seven }, + .{ '8', .eight }, + .{ '9', .nine }, + .{ ';', .semicolon }, + .{ ' ', .space }, + .{ '\'', .apostrophe }, + .{ ',', .comma }, + .{ '`', .grave_accent }, + .{ '.', .period }, + .{ '/', .slash }, + .{ '-', .minus }, + .{ '=', .equal }, + .{ '[', .left_bracket }, + .{ ']', .right_bracket }, + .{ '\\', .backslash }, + + // Keypad entries. We just assume keypad with the kp_ prefix + // so that has some special meaning. These must also always be last. + .{ '0', .kp_0 }, + .{ '1', .kp_1 }, + .{ '2', .kp_2 }, + .{ '3', .kp_3 }, + .{ '4', .kp_4 }, + .{ '5', .kp_5 }, + .{ '6', .kp_6 }, + .{ '7', .kp_7 }, + .{ '8', .kp_8 }, + .{ '9', .kp_9 }, + .{ '.', .kp_decimal }, + .{ '/', .kp_divide }, + .{ '*', .kp_multiply }, + .{ '-', .kp_subtract }, + .{ '+', .kp_add }, + .{ '=', .kp_equal }, + }; };