From d177b20bab4863c56c8d96fa86ab3666dd0de99b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 27 Jan 2024 08:13:53 -0800 Subject: [PATCH 1/4] macos: do not trust AppKit's text translation with ctrl only Normally, when `ctrl+` is pressed, such as `ctrl+z` or `ctrl+c`, macOS (AppKit) doesn't do any key translation because that doesn't map to any printable text on its own. Ghostty does the translation to correctly determine the character is "z" or "c" or whatever. For some reason when the keyboard layout is "Dvorak - QWERTY Cmd" specifically (_not_ plain "Dvorak") on a US layout keyboard, AppKit decides that "ctrl+z" ("/" on a qwerty keyboard) translates to "/"... I can't find any explanation for this. To workaround this, this commit makes it so that if the following conditions are true, then we IGNORE AppKit's text translation and manually do it using UCKeyTranslate: (1) We're on macOS specifically (not iOS, etc.) (2) We have a key event with ONLY control pressed This fixes `ctrl+z` on this unique Dvorak keyboard layout. --- src/apprt/embedded.zig | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index d8e915550..8b6d7c51e 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -765,6 +765,24 @@ pub const Surface = struct { break :translate_mods translate_mods; }; + const event_text: ?[]const u8 = event_text: { + // This logic only applies to macOS. + if (comptime builtin.os.tag != .macos) break :event_text event.text; + + // If the modifiers are ONLY "control" then we never process + // the event text because we want to do our own translation so + // we can handle ctrl+c, ctrl+z, etc. + // + // This is specifically because on macOS using the + // "Dvorak - QWERTY ⌘" keyboard layout, ctrl+z is translated as + // "/" (the physical key that is z on a qwerty keyboard). But on + // other layouts, ctrl+ is not translated by AppKit. So, + // we just avoid this by never allowing AppKit to translate + // ctrl+ and instead do it ourselves. + const ctrl_only = comptime (input.Mods{ .ctrl = true }).int(); + break :event_text if (mods.int() == ctrl_only) null else event.text; + }; + // Translate our key using the keymap for our localized keyboard layout. // We only translate for keydown events. Otherwise, we only care about // the raw keycode. @@ -772,7 +790,7 @@ pub const Surface = struct { const result: input.Keymap.Translation = if (is_down) translate: { // If the event provided us with text, then we use this as a result // and do not do manual translation. - const result: input.Keymap.Translation = if (event.text) |text| .{ + const result: input.Keymap.Translation = if (event_text) |text| .{ .text = text, .composing = event.composing, } else try self.app.keymap.translate( From a41ee3cabec95ae4d52163f8e0c365abd2d0a3f8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 27 Jan 2024 09:58:02 -0800 Subject: [PATCH 2/4] macos: make global macOS fullscreen keybind work Fixes #1389 This is just a fun AppKit quirk. This menu item is set automatically based on the keyboard shortcut (apparently) and when its overwritten then its gone forever. So, let's just not set it. --- macos/Sources/App/macOS/AppDelegate.swift | 7 ++++++- macos/Sources/App/macOS/MainMenu.xib | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index d3d3e1f4b..22ab223dc 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -264,7 +264,6 @@ class AppDelegate: NSObject, syncMenuShortcut(action: "paste_from_clipboard", menuItem: self.menuPaste) syncMenuShortcut(action: "select_all", menuItem: self.menuSelectAll) - syncMenuShortcut(action: "toggle_fullscreen", menuItem: self.menuToggleFullScreen) syncMenuShortcut(action: "toggle_split_zoom", menuItem: self.menuZoomSplit) syncMenuShortcut(action: "goto_split:previous", menuItem: self.menuPreviousSplit) syncMenuShortcut(action: "goto_split:next", menuItem: self.menuNextSplit) @@ -282,6 +281,12 @@ class AppDelegate: NSObject, syncMenuShortcut(action: "decrease_font_size:1", menuItem: self.menuDecreaseFontSize) syncMenuShortcut(action: "reset_font_size", menuItem: self.menuResetFontSize) syncMenuShortcut(action: "inspector:toggle", menuItem: self.menuTerminalInspector) + + // This menu item is NOT synced with the configuration because it disables macOS + // global fullscreen keyboard shortcut. The shortcut in the Ghostty config will continue + // to work but it won't be reflected in the menu item. + // + // syncMenuShortcut(action: "toggle_fullscreen", menuItem: self.menuToggleFullScreen) // Dock menu reloadDockMenu() diff --git a/macos/Sources/App/macOS/MainMenu.xib b/macos/Sources/App/macOS/MainMenu.xib index ca9cd1f35..cd627f762 100644 --- a/macos/Sources/App/macOS/MainMenu.xib +++ b/macos/Sources/App/macOS/MainMenu.xib @@ -234,8 +234,8 @@ - - + + From 9348561bc72c0ff6d42394fdcfff8f7488e6bfb5 Mon Sep 17 00:00:00 2001 From: Selman Kayrancioglu Date: Sat, 27 Jan 2024 17:38:47 +0300 Subject: [PATCH 3/4] config/url: exclude trailing single quotes --- src/config/url.zig | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/config/url.zig b/src/config/url.zig index b0820bb43..0a04ad3f8 100644 --- a/src/config/url.zig +++ b/src/config/url.zig @@ -26,7 +26,7 @@ const oni = @import("oniguruma"); /// handling them well requires a non-regex approach. pub const regex = "(?:" ++ url_scheme ++ ")(?:[^" ++ url_exclude ++ "]*[^" ++ url_exclude ++ ").]|[^" ++ url_exclude ++ "(]*\\([^" ++ url_exclude ++ ")]*\\))"; const url_scheme = "ipfs:|ipns:|magnet:|mailto:|gemini://|gopher://|https://|http://|news:|file:|git://|ssh:|ftp://"; -const url_exclude = "\u{0000}-\u{001F}\u{007F}-\u{009F}<>\x22\\s{-}\\^⟨⟩\x60"; +const url_exclude = "\u{0000}-\u{001F}\u{007F}-\u{009F}<>\x22\x27\\s{-}\\^⟨⟩\x60"; test "url regex" { const testing = std.testing; @@ -67,6 +67,14 @@ test "url regex" { .input = "Link period https://example.com. More text.", .expect = "https://example.com", }, + .{ + .input = "Link in double quotes \"https://example.com\" and more", + .expect = "https://example.com", + }, + .{ + .input = "Link in single quotes 'https://example.com' and more", + .expect = "https://example.com", + }, }; for (cases) |case| { From 0726a8d1faaa10934d3c947bbb4b0624666422d9 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 27 Jan 2024 11:14:16 -0800 Subject: [PATCH 4/4] apprt/embedded: ctrl-only should use binding-mods only This allows it to ignore control side differences. --- src/apprt/embedded.zig | 2 +- src/input/KeyEncoder.zig | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index 8b6d7c51e..511c2da36 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -780,7 +780,7 @@ pub const Surface = struct { // we just avoid this by never allowing AppKit to translate // ctrl+ and instead do it ourselves. const ctrl_only = comptime (input.Mods{ .ctrl = true }).int(); - break :event_text if (mods.int() == ctrl_only) null else event.text; + break :event_text if (mods.binding().int() == ctrl_only) null else event.text; }; // Translate our key using the keymap for our localized keyboard layout. diff --git a/src/input/KeyEncoder.zig b/src/input/KeyEncoder.zig index fc6bc5121..485029677 100644 --- a/src/input/KeyEncoder.zig +++ b/src/input/KeyEncoder.zig @@ -1895,6 +1895,11 @@ test "ctrlseq: normal ctrl c" { try testing.expectEqual(@as(u8, 0x03), seq.?); } +test "ctrlseq: normal ctrl c, right control" { + const seq = ctrlSeq("c", .{ .ctrl = true, .sides = .{ .ctrl = .right } }); + try testing.expectEqual(@as(u8, 0x03), seq.?); +} + test "ctrlseq: alt should be allowed" { const seq = ctrlSeq("c", .{ .alt = true, .ctrl = true }); try testing.expectEqual(@as(u8, 0x03), seq.?);