From 5c952ac977b30f3e4e01417827d4d7745015f50c Mon Sep 17 00:00:00 2001 From: Jon Parise Date: Mon, 17 Aug 2026 10:37:02 -0400 Subject: [PATCH] macos: simplify command palette sort keys Store the Comparable ObjectIdentifier directly instead of wrapping the only sort key type in AnySortKey. The expected deterministic ordering of equal terminal command titles is also now verified by a unit test. --- .../Command Palette/CommandPalette.swift | 4 +-- .../TerminalCommandPalette.swift | 31 ++++++++++--------- macos/Sources/Helpers/AnySortKey.swift | 25 --------------- macos/Tests/CommandPaletteTests.swift | 26 ++++++++++++++-- 4 files changed, 43 insertions(+), 43 deletions(-) delete mode 100644 macos/Sources/Helpers/AnySortKey.swift diff --git a/macos/Sources/Features/Command Palette/CommandPalette.swift b/macos/Sources/Features/Command Palette/CommandPalette.swift index 5a1b5cb36..a7a455632 100644 --- a/macos/Sources/Features/Command Palette/CommandPalette.swift +++ b/macos/Sources/Features/Command Palette/CommandPalette.swift @@ -20,7 +20,7 @@ struct CommandOption: Identifiable, Hashable { /// Whether to visually emphasize this option. let emphasis: Bool /// Sort key for stable ordering when titles are equal. - let sortKey: AnySortKey? + let sortKey: ObjectIdentifier? /// The action to perform when this option is selected. let action: () -> Void @@ -33,7 +33,7 @@ struct CommandOption: Identifiable, Hashable { leadingColor: Color? = nil, badge: String? = nil, emphasis: Bool = false, - sortKey: AnySortKey? = nil, + sortKey: ObjectIdentifier? = nil, action: @escaping () -> Void ) { self.title = title diff --git a/macos/Sources/Features/Command Palette/TerminalCommandPalette.swift b/macos/Sources/Features/Command Palette/TerminalCommandPalette.swift index 4ad690c19..9a01a6e18 100644 --- a/macos/Sources/Features/Command Palette/TerminalCommandPalette.swift +++ b/macos/Sources/Features/Command Palette/TerminalCommandPalette.swift @@ -1,6 +1,21 @@ import SwiftUI import GhosttyKit +func sortedTerminalPaletteOptions(_ options: [CommandOption]) -> [CommandOption] { + options.sorted { lhs, rhs in + let lhsTitle = lhs.title.replacingOccurrences(of: ":", with: "\t") + let rhsTitle = rhs.title.replacingOccurrences(of: ":", with: "\t") + let comparison = lhsTitle.localizedCaseInsensitiveCompare(rhsTitle) + if comparison != .orderedSame { + return comparison == .orderedAscending + } + if let lhsKey = lhs.sortKey, let rhsKey = rhs.sortKey { + return lhsKey < rhsKey + } + return false + } +} + struct TerminalCommandPaletteView: View { /// The surface that this command palette represents. let surfaceView: Ghostty.SurfaceView @@ -64,19 +79,7 @@ struct TerminalCommandPaletteView: View { // Sort the rest. We replace ":" with a character that sorts before space // so that "Foo:" sorts before "Foo Bar:". Use sortKey as a tie-breaker // for stable ordering when titles are equal. - options.append(contentsOf: (jumpOptions + terminalOptions).sorted { a, b in - let aNormalized = a.title.replacingOccurrences(of: ":", with: "\t") - let bNormalized = b.title.replacingOccurrences(of: ":", with: "\t") - let comparison = aNormalized.localizedCaseInsensitiveCompare(bNormalized) - if comparison != .orderedSame { - return comparison == .orderedAscending - } - // Tie-breaker: use sortKey if both have one - if let aSortKey = a.sortKey, let bSortKey = b.sortKey { - return aSortKey < bSortKey - } - return false - }) + options.append(contentsOf: sortedTerminalPaletteOptions(jumpOptions + terminalOptions)) return options } @@ -168,7 +171,7 @@ struct TerminalCommandPaletteView: View { subtitle: subtitle, leadingIcon: "rectangle.on.rectangle", leadingColor: displayColor?.displayColor.map { Color($0) }, - sortKey: AnySortKey(ObjectIdentifier(surface)) + sortKey: ObjectIdentifier(surface) ) { NotificationCenter.default.post( name: Ghostty.Notification.ghosttyPresentTerminal, diff --git a/macos/Sources/Helpers/AnySortKey.swift b/macos/Sources/Helpers/AnySortKey.swift deleted file mode 100644 index ffafb6b90..000000000 --- a/macos/Sources/Helpers/AnySortKey.swift +++ /dev/null @@ -1,25 +0,0 @@ -import Foundation - -/// Type-erased wrapper for any Comparable type to use as a sort key. -struct AnySortKey: Comparable { - private let value: Any - private let comparator: (Any, Any) -> ComparisonResult - - init(_ value: T) { - self.value = value - self.comparator = { lhs, rhs in - guard let l = lhs as? T, let r = rhs as? T else { return .orderedSame } - if l < r { return .orderedAscending } - if l > r { return .orderedDescending } - return .orderedSame - } - } - - static func < (lhs: AnySortKey, rhs: AnySortKey) -> Bool { - lhs.comparator(lhs.value, rhs.value) == .orderedAscending - } - - static func == (lhs: AnySortKey, rhs: AnySortKey) -> Bool { - lhs.comparator(lhs.value, rhs.value) == .orderedSame - } -} diff --git a/macos/Tests/CommandPaletteTests.swift b/macos/Tests/CommandPaletteTests.swift index fc36ef9fa..1f4b45f78 100644 --- a/macos/Tests/CommandPaletteTests.swift +++ b/macos/Tests/CommandPaletteTests.swift @@ -14,13 +14,15 @@ struct CommandPaletteFilterTests { title: String, subtitle: String? = nil, description: String? = nil, - leadingColor: Color? = nil + leadingColor: Color? = nil, + sortKey: ObjectIdentifier? = nil ) -> CommandOption { CommandOption( title: title, subtitle: subtitle, description: description, - leadingColor: leadingColor + leadingColor: leadingColor, + sortKey: sortKey ) {} } @@ -46,4 +48,24 @@ struct CommandPaletteFilterTests { #expect([first, second].filteredAndSorted(query: "new") == [first, second]) #expect([second, first].filteredAndSorted(query: "new") == [second, first]) } + + /// Equal titles use their sort keys independent of input order. + @Test func equalTitlesUseSortKey() { + let firstKey = NSObject() + let secondKey = NSObject() + let first = option( + title: "Focus: Shell", + subtitle: "/tmp", + sortKey: ObjectIdentifier(firstKey) + ) + let second = option( + title: "Focus: Shell", + subtitle: "/tmp", + sortKey: ObjectIdentifier(secondKey) + ) + + let forward = sortedTerminalPaletteOptions([first, second]) + let reverse = sortedTerminalPaletteOptions([second, first]) + #expect(forward == reverse) + } }