From 18f270222557fd46d8c3305da523212445066154 Mon Sep 17 00:00:00 2001 From: Lukas <134181853+bo2themax@users.noreply.github.com> Date: Thu, 2 Apr 2026 19:33:27 +0200 Subject: [PATCH] macOS: fix Find Next/Previous button in the menu bar is not working as expected --- macos/Sources/App/macOS/AppDelegate.swift | 4 +- .../Ghostty/Surface View/SurfaceView.swift | 38 +++++++++++++++---- .../Surface View/SurfaceView_AppKit.swift | 12 +----- macos/Tests/Ghostty/ConfigTests.swift | 11 ++++++ src/config/CApi.zig | 36 ++++++++++++++++++ 5 files changed, 81 insertions(+), 20 deletions(-) diff --git a/macos/Sources/App/macOS/AppDelegate.swift b/macos/Sources/App/macOS/AppDelegate.swift index 645915ae1..505d74f7e 100644 --- a/macos/Sources/App/macOS/AppDelegate.swift +++ b/macos/Sources/App/macOS/AppDelegate.swift @@ -1171,8 +1171,8 @@ extension AppDelegate { syncMenuShortcut(config, action: "start_search", menuItem: self.menuFind) syncMenuShortcut(config, action: "search_selection", menuItem: self.menuSelectionForFind) syncMenuShortcut(config, action: "scroll_to_selection", menuItem: self.menuScrollToSelection) - syncMenuShortcut(config, action: "search:next", menuItem: self.menuFindNext) - syncMenuShortcut(config, action: "search:previous", menuItem: self.menuFindPrevious) + syncMenuShortcut(config, action: "navigate_search:next", menuItem: self.menuFindNext) + syncMenuShortcut(config, action: "navigate_search:previous", menuItem: self.menuFindPrevious) syncMenuShortcut(config, action: "toggle_split_zoom", menuItem: self.menuZoomSplit) syncMenuShortcut(config, action: "goto_split:previous", menuItem: self.menuPreviousSplit) diff --git a/macos/Sources/Ghostty/Surface View/SurfaceView.swift b/macos/Sources/Ghostty/Surface View/SurfaceView.swift index 47503dc0e..6323e6af6 100644 --- a/macos/Sources/Ghostty/Surface View/SurfaceView.swift +++ b/macos/Sources/Ghostty/Surface View/SurfaceView.swift @@ -446,18 +446,16 @@ extension Ghostty { } #endif .backport.onKeyPress(.return) { modifiers in - guard let surface = surfaceView.surface else { return .ignored } - let action = modifiers.contains(.shift) - ? "navigate_search:previous" - : "navigate_search:next" - ghostty_surface_binding_action(surface, action, UInt(action.lengthOfBytes(using: .utf8))) + if modifiers.contains(.shift) { + _ = surfaceView.navigateSearchToPrevious() + } else { + _ = surfaceView.navigateSearchToNext() + } return .handled } Button(action: { - guard let surface = surfaceView.surface else { return } - let action = "navigate_search:next" - ghostty_surface_binding_action(surface, action, UInt(action.lengthOfBytes(using: .utf8))) + _ = surfaceView.navigateSearchToNext() }, label: { Image(systemName: "chevron.up") }) @@ -1277,4 +1275,28 @@ extension Ghostty.SurfaceView { self.needle = startSearch.needle ?? "" } } + + func navigateSearchToNext() -> Bool { + guard let surface = self.surface else { return false } + let action = "navigate_search:next" + if !ghostty_surface_binding_action(surface, action, UInt(action.lengthOfBytes(using: .utf8))) { +#if canImport(AppKit) + AppDelegate.logger.warning("action failed action=\(action)") +#endif + return false + } + return true + } + + func navigateSearchToPrevious() -> Bool { + guard let surface = self.surface else { return false } + let action = "navigate_search:previous" + if !ghostty_surface_binding_action(surface, action, UInt(action.lengthOfBytes(using: .utf8))) { +#if canImport(AppKit) + AppDelegate.logger.warning("action failed action=\(action)") +#endif + return false + } + return true + } } diff --git a/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift b/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift index 8f4fb01cf..f9448cd0d 100644 --- a/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift +++ b/macos/Sources/Ghostty/Surface View/SurfaceView_AppKit.swift @@ -1604,19 +1604,11 @@ extension Ghostty { } @IBAction func findNext(_ sender: Any?) { - guard let surface = self.surface else { return } - let action = "search:next" - if !ghostty_surface_binding_action(surface, action, UInt(action.lengthOfBytes(using: .utf8))) { - AppDelegate.logger.warning("action failed action=\(action)") - } + _ = self.navigateSearchToNext() } @IBAction func findPrevious(_ sender: Any?) { - guard let surface = self.surface else { return } - let action = "search:previous" - if !ghostty_surface_binding_action(surface, action, UInt(action.lengthOfBytes(using: .utf8))) { - AppDelegate.logger.warning("action failed action=\(action)") - } + _ = navigateSearchToPrevious() } @IBAction func findHide(_ sender: Any?) { diff --git a/macos/Tests/Ghostty/ConfigTests.swift b/macos/Tests/Ghostty/ConfigTests.swift index 9581efd56..a4b8472ac 100644 --- a/macos/Tests/Ghostty/ConfigTests.swift +++ b/macos/Tests/Ghostty/ConfigTests.swift @@ -221,6 +221,8 @@ struct ConfigTests { #expect(config.focusFollowsMouse == true) } + // MARK: - Keybind + @Test func uppercasedLetterShouldBeNormalized() async throws { let config = try TemporaryConfig(""" @@ -235,4 +237,13 @@ struct ConfigTests { let shortcut2 = try #require(config2.keyboardShortcut(for: "goto_split:left")) #expect(shortcut2 == .init("รค", modifiers: [.command])) } + + @Test + func emptyConfigShouldBeHaveDefaultShortcut() async throws { + let config = try TemporaryConfig("") + let newWindow = try #require(config.keyboardShortcut(for: "new_window")) + #expect(newWindow == .init("n", modifiers: [.command])) + let gotoToNextSplit = try #require(config.keyboardShortcut(for: "goto_split:next")) + #expect(gotoToNextSplit == .init("]", modifiers: [.command])) + } } diff --git a/src/config/CApi.zig b/src/config/CApi.zig index ca15ce4e8..6e835b4a4 100644 --- a/src/config/CApi.zig +++ b/src/config/CApi.zig @@ -1,3 +1,4 @@ +const builtin = @import("builtin"); const std = @import("std"); const inputpkg = @import("../input.zig"); const state = &@import("../global.zig").state; @@ -242,3 +243,38 @@ test "ghostty_config_get: struct cval conversion" { try testing.expectEqual(@as(u8, 34), out.g); try testing.expectEqual(@as(u8, 56), out.b); } + +test "ghostty_config_trigger: default keybind" { + const testing = std.testing; + + var cfg = try Config.default(testing.allocator); + defer cfg.deinit(); + + // Default commands should be fetchable through config_trigger_ + { + const trigger = try config_trigger_(&cfg, "open_config"); + try testing.expectEqual(.unicode, trigger.tag); + try testing.expectEqual(@as(u32, ','), trigger.key.unicode); + } + { + const trigger = try config_trigger_(&cfg, "reload_config"); + try testing.expectEqual(.unicode, trigger.tag); + try testing.expectEqual(@as(u32, ','), trigger.key.unicode); + } + // Performable bindings are not tracked in the reverse map, + // so config_trigger_ should return a default (empty) trigger. + if (comptime builtin.target.os.tag.isDarwin()) { + const next = try config_trigger_(&cfg, "navigate_search:next"); + try testing.expectEqual(.physical, next.tag); + try testing.expectEqual(.unidentified, next.key.physical); + + const prev = try config_trigger_(&cfg, "navigate_search:previous"); + try testing.expectEqual(.physical, prev.tag); + try testing.expectEqual(.unidentified, prev.key.physical); + } + { + const trigger = try config_trigger_(&cfg, "adjust_selection:left"); + try testing.expectEqual(.physical, trigger.tag); + try testing.expectEqual(.unidentified, trigger.key.physical); + } +}