Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import Foundation

/// Returns the canonical key for a newly recorded macOS key event.
///
/// macOS reports the shifted glyph in `charactersIgnoringModifiers` for
/// punctuation keys (`<`, `>`, `{`, …). cmux stores the physical base key and
/// the Shift modifier separately so recording, conflict detection, menus, and
/// runtime matching all describe the same keystroke.
public func recordedShortcutKey(
keyCode: UInt16,
charactersIgnoringModifiers: String?
) -> String? {
if let physicalKey = physicalBaseShortcutKey(for: keyCode) {
return physicalKey
}

guard let characters = charactersIgnoringModifiers?.lowercased(),
!characters.isEmpty,
characters.unicodeScalars.allSatisfy({
!CharacterSet.controlCharacters.contains($0)
}) else {
return nil
}
return characters
}

/// Normalizes an already stored key when its recording-time key code is
/// available. Hand-written bindings without a key code retain their logical
/// key so non-US layouts remain configurable.
public func canonicalShortcutKey(_ key: String, keyCode: UInt16?) -> String {
if key.lowercased().hasPrefix("media.") {
return key
}
guard let keyCode,
let physicalKey = physicalBaseShortcutKey(for: keyCode) else {
return key.lowercased()
}
return physicalKey
}

private func physicalBaseShortcutKey(for keyCode: UInt16) -> String? {
switch keyCode {
case 36, 76: return "\r"
case 48: return "\t"
case 49: return "space"
case 18: return "1"
case 19: return "2"
case 20: return "3"
case 21: return "4"
case 23: return "5"
case 22: return "6"
case 26: return "7"
case 28: return "8"
case 25: return "9"
case 29: return "0"
case 24: return "="
case 27: return "-"
case 30: return "]"
case 33: return "["
case 39: return "'"
case 41: return ";"
case 42: return "\\"
case 43: return ","
case 44: return "/"
case 47: return "."
case 50: return "`"
case 123: return "←"
case 124: return "→"
case 125: return "↓"
case 126: return "↑"
case 122: return "f1"
case 120: return "f2"
case 99: return "f3"
case 118: return "f4"
case 96: return "f5"
case 97: return "f6"
case 98: return "f7"
case 100: return "f8"
case 101: return "f9"
case 109: return "f10"
case 103: return "f11"
case 111: return "f12"
case 105: return "f13"
case 107: return "f14"
case 113: return "f15"
case 106: return "f16"
case 64: return "f17"
case 79: return "f18"
case 80: return "f19"
case 90: return "f20"
default: return nil
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,17 @@ public struct ShortcutStroke: Sendable, Equatable, Hashable, Codable {

/// True when at least one of `cmd`, `shift`, `opt`, or `ctrl` is set.
public var hasAnyModifier: Bool { command || shift || option || control }

/// Returns this stroke with its key normalized to cmux's persisted
/// physical-key representation when a recording-time key code is present.
public func canonicalized() -> ShortcutStroke {
ShortcutStroke(
key: canonicalShortcutKey(key, keyCode: keyCode),
command: command,
shift: shift,
option: option,
control: control,
keyCode: keyCode
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ public struct StoredShortcut: Sendable, Equatable, Hashable, Codable, SettingCod
/// True when the binding fires on two consecutive strokes.
public var hasChord: Bool { second != nil }

/// Returns the binding with both strokes using canonical persisted keys.
public func canonicalized() -> StoredShortcut {
StoredShortcut(first: first.canonicalized(), second: second?.canonicalized())
}

// MARK: - SettingCodable

public static func decodeFromUserDefaults(_ raw: Any?) -> StoredShortcut? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ final class ShortcutListModel {
/// another binding; a valid stroke is normalized, persisted, and clears the
/// action's rejection/restore state.
func assign(stroke: ShortcutStroke, to action: ShortcutAction) async {
var stroke = stroke
var stroke = stroke.canonicalized()
guard action.allowsBareFirstStroke || stroke.hasAnyModifier else {
markBareKeyRejected(action)
return
Expand Down Expand Up @@ -338,6 +338,7 @@ final class ShortcutListModel {
/// action that disallows chords, a non-digit numbered chord, or a chord that
/// conflicts with another binding.
func assignChord(_ chord: StoredShortcut, to action: ShortcutAction) async {
let chord = chord.canonicalized()
guard action.allowsChordShortcut else {
chordModeActions.remove(action.rawValue)
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,15 +369,18 @@ public final class RecorderHostButton: NSButton {
return
}

guard let chars = event.charactersIgnoringModifiers, !chars.isEmpty else { return }
guard let key = recordedShortcutKey(
keyCode: event.keyCode,
charactersIgnoringModifiers: event.charactersIgnoringModifiers
) else { return }

let hasModifier = event.modifierFlags.contains(.command)
|| event.modifierFlags.contains(.option)
|| event.modifierFlags.contains(.control)
|| event.modifierFlags.contains(.shift)

let stroke = ShortcutStroke(
key: chars.lowercased(),
key: key,
command: event.modifierFlags.contains(.command),
shift: event.modifierFlags.contains(.shift),
option: event.modifierFlags.contains(.option),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ func numberedAwareStrokesConflict(
_ rhs: ShortcutStroke,
numbered rhsNumbered: Bool
) -> Bool {
let lhs = lhs.canonicalized()
let rhs = rhs.canonicalized()
let lhsFamily = lhsNumbered && isNumberedDigitKey(lhs.key)
let rhsFamily = rhsNumbered && isNumberedDigitKey(rhs.key)
if lhsFamily || rhsFamily {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,77 @@ struct ShortcutRecorderViewTests {
#expect(!button.isRecording)
}

private func keyDownEvent(key: String, keyCode: UInt16) throws -> NSEvent {
@Test func commandShiftLessThanIsRejectedAsReloadConfigurationCollision() async throws {
let tempDirectory = FileManager.default.temporaryDirectory
.appendingPathComponent("shortcut-recorder-collision-\(UUID().uuidString)", isDirectory: true)
try FileManager.default.createDirectory(at: tempDirectory, withIntermediateDirectories: true)
defer { try? FileManager.default.removeItem(at: tempDirectory) }

let button = RecorderHostButton(frame: .zero)
defer {
if button.isRecording {
button.stopRecording()
}
}
var recordedStroke: ShortcutStroke?
button.onStroke = { recordedStroke = $0 }
button.startRecording()
button.handleRecordingEvent(try keyDownEvent(
key: "<",
keyCode: 43,
modifierFlags: [.command, .shift]
))

let stroke = try #require(recordedStroke)
let store = JSONConfigStore(fileURL: tempDirectory.appendingPathComponent("cmux.json"))
let catalog = SettingCatalog()
let model = ShortcutListModel(
jsonStore: store,
catalog: catalog,
errorLog: SettingsErrorLog()
)

await model.assign(stroke: stroke, to: .focusHistoryBack)

let bindings = await store.value(for: catalog.shortcuts.bindings)
#expect(bindings[ShortcutAction.focusHistoryBack.rawValue] == nil)
#expect(
model.conflictRejections[ShortcutAction.focusHistoryBack.rawValue]
== .reloadConfiguration
)
}

@Test func unmappedPrintableSymbolRemainsRecordable() throws {
let button = RecorderHostButton(frame: .zero)
defer {
if button.isRecording {
button.stopRecording()
}
}
var recordedStroke: ShortcutStroke?
button.onStroke = { recordedStroke = $0 }
button.startRecording()

button.handleRecordingEvent(try keyDownEvent(
key: "§",
keyCode: 10,
modifierFlags: [.command]
))

#expect(recordedStroke?.key == "§")
#expect(recordedStroke?.keyCode == 10)
}

private func keyDownEvent(
key: String,
keyCode: UInt16,
modifierFlags: NSEvent.ModifierFlags = []
) throws -> NSEvent {
try #require(
NSEvent.keyEvent(
with: .keyDown,
location: .zero,
modifierFlags: [],
modifierFlags: modifierFlags,
timestamp: ProcessInfo.processInfo.systemUptime,
windowNumber: 0,
context: nil,
Expand Down
43 changes: 20 additions & 23 deletions Sources/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12918,6 +12918,14 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
return false
}

if shortcutRoutingShouldBypassForPrintableOptionText(event: event) {
let shortcutWindow = resolvedShortcutEventWindow(event) ?? shortcutRoutingActiveWindow
if browserResponderHasMarkedText(shortcutWindow?.firstResponder) {
clearConfiguredShortcutChordState()
return false
}
}

// `charactersIgnoringModifiers` can be nil for some synthetic NSEvents and certain special keys.
// Treat nil as "" and rely on keyCode/layout-aware fallback logic where needed.
// When a non-Latin input source is active (Korean, Chinese, Japanese, etc.),
Expand Down Expand Up @@ -13306,10 +13314,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
return true
}

if shouldBypassPrintableOptionTextForShortcutRouting(event: event) {
return false
}

let canvasSurfaceDigitShortcutIsActive =
shortcutEventFocusContext(event).shortcutContext.bool(ShortcutContextKnownKey.workspaceCanvasLayout.rawValue) &&
shortcutWhenClauseAllows(action: .selectSurfaceByNumber, event: event) &&
Expand Down Expand Up @@ -15194,7 +15198,12 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
/// Resolves a right-sidebar mode shortcut after applying the action's
/// effective `when` clause.
func rightSidebarModeShortcut(for event: NSEvent) -> RightSidebarMode? {
KeyboardShortcutSettingsObserver.shared.rightSidebarModeShortcutMatcher.modeShortcut(for: event) { [self] action in
let shortcutWindow = resolvedShortcutEventWindow(event) ?? event.window ?? shortcutRoutingActiveWindow
if shortcutRoutingShouldBypassForPrintableOptionText(event: event),
browserResponderHasMarkedText(shortcutWindow?.firstResponder) {
return nil
}
return KeyboardShortcutSettingsObserver.shared.rightSidebarModeShortcutMatcher.modeShortcut(for: event) { [self] action in
shortcutWhenClauseAllows(action: action, event: event)
}
}
Expand Down Expand Up @@ -15232,22 +15241,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
return nil
}

fileprivate func shouldBypassPrintableOptionTextForShortcutRouting(event: NSEvent) -> Bool {
guard shortcutRoutingShouldBypassForPrintableOptionText(event: event) else {
return false
}

if routableNumberedConfiguredShortcutDigit(event: event, action: .selectWorkspaceByNumber) != nil {
return false
}

if routableNumberedConfiguredShortcutDigit(event: event, action: .selectSurfaceByNumber) != nil {
return false
}

return true
}

private func tabManagerForNumberedShortcut(event: NSEvent) -> TabManager? {
preferredMainWindowContextForShortcutRouting(event: event)?.tabManager ?? tabManager
}
Expand Down Expand Up @@ -17075,13 +17068,17 @@ private extension NSWindow {
return true
}
let browserWebKitKeyDownReentry = firstResponderWebView != nil && cmuxBrowserWebKitKeyDownDispatchIsActive()
if AppDelegate.shared?.shouldBypassPrintableOptionTextForShortcutRouting(event: event) == true {
if shortcutRoutingShouldBypassForPrintableOptionText(event: event) {
if browserWebKitKeyDownReentry { return false }
if !firstResponderHasMarkedText,
AppDelegate.shared?.handleConfiguredShortcutKeyEquivalent(event) == true {
return true
}
let textInputTarget: NSResponder? = firstResponderGhosttyView
?? firstResponderWebView
?? self.firstResponder
if let textInputTarget, textInputTarget !== self {
if cmuxForceDispatchKeyDownOnce(event, to: textInputTarget, reason: "printable Option text") {
if cmuxForceDispatchKeyDownOnce(event, to: textInputTarget, reason: "unmatched Option input") {
return true
}
// Same event already in flight on this stack (WebKit replay /
Expand Down
44 changes: 8 additions & 36 deletions Sources/KeyboardShortcutSettings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1663,9 +1663,6 @@ struct ShortcutStroke: Equatable, Hashable {
}

guard event.type == .keyDown else { return false }
if shortcutRoutingShouldBypassForPrintableOptionText(event: event) {
return false
}

return matches(
keyCode: Self.recordableKey(from: event)?.keyCode ?? event.keyCode,
Expand All @@ -1684,6 +1681,10 @@ struct ShortcutStroke: Equatable, Hashable {
let flags = Self.normalizedModifierFlags(from: modifierFlags)
guard flags == self.modifierFlags else { return false }

if let recordedKeyCode = self.keyCode {
return keyCode == recordedKeyCode
}

let shortcutKey = key.lowercased()
if Self.usesDirectKeyCodeMatching(shortcutKey) {
guard let expectedKeyCode = self.keyCode ?? Self.keyCodeForShortcutKey(shortcutKey) else {
Expand Down Expand Up @@ -1786,39 +1787,10 @@ struct ShortcutStroke: Equatable, Hashable {
keyCode: UInt16,
charactersIgnoringModifiers: String?
) -> String? {
// Prefer keyCode mapping so shifted symbol keys (e.g. "}") record as "]".
switch keyCode {
case 123: return "←" // left arrow
case 124: return "→" // right arrow
case 125: return "↓" // down arrow
case 126: return "↑" // up arrow
case 48: return "\t" // tab
case 49: return "space" // kVK_Space
case 36, 76: return "\r" // return, keypad enter
case 33: return "[" // kVK_ANSI_LeftBracket
case 30: return "]" // kVK_ANSI_RightBracket
case 27: return "-" // kVK_ANSI_Minus
case 24: return "=" // kVK_ANSI_Equal
case 43: return "," // kVK_ANSI_Comma
case 47: return "." // kVK_ANSI_Period
case 44: return "/" // kVK_ANSI_Slash
case 41: return ";" // kVK_ANSI_Semicolon
case 39: return "'" // kVK_ANSI_Quote
case 50: return "`" // kVK_ANSI_Grave
case 42: return "\\" // kVK_ANSI_Backslash
default:
break
}

guard let chars = charactersIgnoringModifiers?.lowercased(),
let char = chars.first else {
return nil
}

if char.isLetter || char.isNumber {
return String(char)
}
return nil
recordedShortcutKey(
keyCode: keyCode,
charactersIgnoringModifiers: charactersIgnoringModifiers
)
}

private static func recordableKey(
Expand Down
Loading