fix: better handle some shortcut conflicts (related to 3059dc23)
This commit is contained in:
lwouis committed
1 parent
83b5ddc091
commit
cddc89dd21
30 files changed
+181
-87
No files matched your search
@@ -656,7 +656,7 @@
|
||||
"Shortcut" = "Shortcut";
|
||||
|
||||
/* No comment provided by engineer. */
|
||||
"Shortcut already assigned to another action: %@" = "Shortcut already assigned to another action: %@";
|
||||
"Shortcut already assigned to: %@" = "Shortcut already assigned to: %@";
|
||||
|
||||
/* No comment provided by engineer. */
|
||||
"Shortcuts When Active" = "Shortcuts When Active";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "تم استيراد الإعدادات";
|
||||
"Settings…" = "الإعدادات…";
|
||||
"Shortcut" = "اختصار";
|
||||
"Shortcut already assigned to another action: %@" = "الاختصار معيّن بالفعل لإجراء آخر: %@";
|
||||
"Shortcut already assigned to: %@" = "الاختصار معيّن بالفعل: %@";
|
||||
"Shortcuts When Active" = "الاختصارات عند النشاط";
|
||||
"Shortcuts when active…" = "الاختصارات عند النشاط…";
|
||||
"Show" = "إظهار";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "Einstellungen importiert";
|
||||
"Settings…" = "Einstellungen…";
|
||||
"Shortcut" = "Tastenkürzel";
|
||||
"Shortcut already assigned to another action: %@" = "Tastenkürzel ist bereits einer anderen Aktion zugewiesen: %@";
|
||||
"Shortcut already assigned to: %@" = "Tastenkürzel ist bereits zugewiesen: %@";
|
||||
"Shortcuts When Active" = "Tastenkürzel bei Aktivität";
|
||||
"Shortcuts when active…" = "Tastenkürzel bei Aktivität…";
|
||||
"Show" = "Anzeigen";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "Settings imported";
|
||||
"Settings…" = "Settings…";
|
||||
"Shortcut" = "Shortcut";
|
||||
"Shortcut already assigned to another action: %@" = "Shortcut already assigned to another action: %@";
|
||||
"Shortcut already assigned to: %@" = "Shortcut already assigned to: %@";
|
||||
"Shortcuts When Active" = "Shortcuts When Active";
|
||||
"Shortcuts when active…" = "Shortcuts when active…";
|
||||
"Show" = "Show";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "Ajustes importados";
|
||||
"Settings…" = "Ajustes…";
|
||||
"Shortcut" = "Atajo";
|
||||
"Shortcut already assigned to another action: %@" = "Atajo ya asignado a otra acción: %@";
|
||||
"Shortcut already assigned to: %@" = "Atajo ya asignado a: %@";
|
||||
"Shortcuts When Active" = "Atajos cuando está activo";
|
||||
"Shortcuts when active…" = "Atajos cuando está activo…";
|
||||
"Show" = "Mostrar";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "Réglages importés";
|
||||
"Settings…" = "Réglages…";
|
||||
"Shortcut" = "Raccourci";
|
||||
"Shortcut already assigned to another action: %@" = "Raccourci déjà affecté à une autre action : %@";
|
||||
"Shortcut already assigned to: %@" = "Raccourci déjà affecté à : %@";
|
||||
"Shortcuts When Active" = "Raccourcis pendant l'utilisation";
|
||||
"Shortcuts when active…" = "Raccourcis pendant l'utilisation…";
|
||||
"Show" = "Afficher";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "ההגדרות יובאו";
|
||||
"Settings…" = "הגדרות…";
|
||||
"Shortcut" = "קיצור דרך";
|
||||
"Shortcut already assigned to another action: %@" = "קיצור הדרך כבר משויך לפעולה אחרת: %@";
|
||||
"Shortcut already assigned to: %@" = "קיצור הדרך כבר משויך: %@";
|
||||
"Shortcuts When Active" = "קיצורי דרך כאשר פעיל";
|
||||
"Shortcuts when active…" = "קיצורי דרך כאשר פעיל…";
|
||||
"Show" = "הצג";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "Pengaturan diimpor";
|
||||
"Settings…" = "Pengaturan…";
|
||||
"Shortcut" = "Pintasan";
|
||||
"Shortcut already assigned to another action: %@" = "Pintasan sudah ditetapkan untuk tindakan lain: %@";
|
||||
"Shortcut already assigned to: %@" = "Pintasan sudah ditetapkan untuk: %@";
|
||||
"Shortcuts When Active" = "Pintasan Saat Aktif";
|
||||
"Shortcuts when active…" = "Pintasan saat aktif…";
|
||||
"Show" = "Tampilkan";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "Impostazioni importate";
|
||||
"Settings…" = "Impostazioni…";
|
||||
"Shortcut" = "Scorciatoia";
|
||||
"Shortcut already assigned to another action: %@" = "Scorciatoia già assegnata a un'altra azione: %@";
|
||||
"Shortcut already assigned to: %@" = "Scorciatoia già assegnata a: %@";
|
||||
"Shortcuts When Active" = "Scorciatoie quando attivo";
|
||||
"Shortcuts when active…" = "Scorciatoie quando attivo…";
|
||||
"Show" = "Mostra";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "設定を読み込みました";
|
||||
"Settings…" = "設定…";
|
||||
"Shortcut" = "ショートカット";
|
||||
"Shortcut already assigned to another action: %@" = "ショートカットはすでに別のアクションに割り当てられています: %@";
|
||||
"Shortcut already assigned to: %@" = "ショートカットはすでに割り当てられています: %@";
|
||||
"Shortcuts When Active" = "アクティブ時のショートカット";
|
||||
"Shortcuts when active…" = "アクティブ時のショートカット…";
|
||||
"Show" = "表示";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "설정을 가져왔습니다";
|
||||
"Settings…" = "설정…";
|
||||
"Shortcut" = "단축키";
|
||||
"Shortcut already assigned to another action: %@" = "단축키가 이미 다른 동작에 할당되어 있습니다: %@";
|
||||
"Shortcut already assigned to: %@" = "단축키가 이미 할당되어 있습니다: %@";
|
||||
"Shortcuts When Active" = "활성 시 단축키";
|
||||
"Shortcuts when active…" = "활성 시 단축키…";
|
||||
"Show" = "보이기";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "Instellingen geïmporteerd";
|
||||
"Settings…" = "Instellingen…";
|
||||
"Shortcut" = "Sneltoets";
|
||||
"Shortcut already assigned to another action: %@" = "Sneltoets is al toegewezen aan een andere actie: %@";
|
||||
"Shortcut already assigned to: %@" = "Sneltoets is al toegewezen aan: %@";
|
||||
"Shortcuts When Active" = "Sneltoetsen wanneer actief";
|
||||
"Shortcuts when active…" = "Sneltoetsen wanneer actief…";
|
||||
"Show" = "Toon";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "Ustawienia zaimportowane";
|
||||
"Settings…" = "Ustawienia…";
|
||||
"Shortcut" = "Skrót";
|
||||
"Shortcut already assigned to another action: %@" = "Skrót jest już przypisany do innej akcji: %@";
|
||||
"Shortcut already assigned to: %@" = "Skrót jest już przypisany do: %@";
|
||||
"Shortcuts When Active" = "Skróty, gdy aktywne";
|
||||
"Shortcuts when active…" = "Skróty, gdy aktywne…";
|
||||
"Show" = "Pokaż";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "Ajustes importados";
|
||||
"Settings…" = "Ajustes…";
|
||||
"Shortcut" = "Atalho";
|
||||
"Shortcut already assigned to another action: %@" = "Atalho já atribuído a outra ação: %@";
|
||||
"Shortcut already assigned to: %@" = "Atalho já atribuído a: %@";
|
||||
"Shortcuts When Active" = "Atalhos quando ativo";
|
||||
"Shortcuts when active…" = "Atalhos quando ativo…";
|
||||
"Show" = "Mostrar";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "Настройки импортированы";
|
||||
"Settings…" = "Настройки…";
|
||||
"Shortcut" = "Сочетание клавиш";
|
||||
"Shortcut already assigned to another action: %@" = "Сочетание клавиш уже назначено другому действию: %@";
|
||||
"Shortcut already assigned to: %@" = "Сочетание клавиш уже назначено: %@";
|
||||
"Shortcuts When Active" = "Сочетания клавиш при активном переключателе";
|
||||
"Shortcuts when active…" = "Сочетания клавиш при активном переключателе…";
|
||||
"Show" = "Показать";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "Inställningar importerade";
|
||||
"Settings…" = "Inställningar…";
|
||||
"Shortcut" = "Kortkommando";
|
||||
"Shortcut already assigned to another action: %@" = "Kortkommandot är redan tilldelat en annan åtgärd: %@";
|
||||
"Shortcut already assigned to: %@" = "Kortkommandot är redan tilldelat: %@";
|
||||
"Shortcuts When Active" = "Kortkommandon när aktiv";
|
||||
"Shortcuts when active…" = "Kortkommandon när aktiv…";
|
||||
"Show" = "Visa";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "นำเข้าการตั้งค่าแล้ว";
|
||||
"Settings…" = "การตั้งค่า…";
|
||||
"Shortcut" = "ทางลัด";
|
||||
"Shortcut already assigned to another action: %@" = "ทางลัดถูกกำหนดให้กับการกระทำอื่นแล้ว: %@";
|
||||
"Shortcut already assigned to: %@" = "ทางลัดถูกกำหนดไว้แล้ว: %@";
|
||||
"Shortcuts When Active" = "ทางลัดเมื่อทำงานอยู่";
|
||||
"Shortcuts when active…" = "ทางลัดเมื่อทำงานอยู่…";
|
||||
"Show" = "แสดง";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "Ayarlar içe aktarıldı";
|
||||
"Settings…" = "Ayarlar…";
|
||||
"Shortcut" = "Kısayol";
|
||||
"Shortcut already assigned to another action: %@" = "Kısayol zaten başka bir eyleme atanmış: %@";
|
||||
"Shortcut already assigned to: %@" = "Kısayol zaten şuna atanmış: %@";
|
||||
"Shortcuts When Active" = "Etkinken Kullanılan Kısayollar";
|
||||
"Shortcuts when active…" = "Etkinken kullanılan kısayollar…";
|
||||
"Show" = "Göster";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "Đã nhập cài đặt";
|
||||
"Settings…" = "Cài đặt…";
|
||||
"Shortcut" = "Phím tắt";
|
||||
"Shortcut already assigned to another action: %@" = "Phím tắt đã được gán cho một hành động khác: %@";
|
||||
"Shortcut already assigned to: %@" = "Phím tắt đã được gán cho: %@";
|
||||
"Shortcuts When Active" = "Phím tắt khi hoạt động";
|
||||
"Shortcuts when active…" = "Phím tắt khi hoạt động…";
|
||||
"Show" = "Hiển thị";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "设置已导入";
|
||||
"Settings…" = "设置…";
|
||||
"Shortcut" = "快捷键";
|
||||
"Shortcut already assigned to another action: %@" = "快捷键已分配给其他操作：%@";
|
||||
"Shortcut already assigned to: %@" = "快捷键已分配给：%@";
|
||||
"Shortcuts When Active" = "激活时的快捷键";
|
||||
"Shortcuts when active…" = "激活时的快捷键…";
|
||||
"Show" = "显示";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "已輸入設定";
|
||||
"Settings…" = "設定…";
|
||||
"Shortcut" = "快捷鍵";
|
||||
"Shortcut already assigned to another action: %@" = "快捷鍵已指派給另一項動作:%@";
|
||||
"Shortcut already assigned to: %@" = "快捷鍵已指派給:%@";
|
||||
"Shortcuts When Active" = "作用中時的快捷鍵";
|
||||
"Shortcuts when active…" = "作用中時的快捷鍵…";
|
||||
"Show" = "顯示";
|
||||
|
||||
@@ -217,7 +217,7 @@
|
||||
"Settings imported" = "設定已輸入";
|
||||
"Settings…" = "設定…";
|
||||
"Shortcut" = "快速鍵";
|
||||
"Shortcut already assigned to another action: %@" = "快速鍵已指派給其他動作：%@";
|
||||
"Shortcut already assigned to: %@" = "快速鍵已指派給：%@";
|
||||
"Shortcuts When Active" = "啟用時的快速鍵";
|
||||
"Shortcuts when active…" = "啟用時的快速鍵…";
|
||||
"Show" = "顯示";
|
||||
|
||||
@@ -220,6 +220,8 @@ class Preferences {
|
||||
static var holdShortcut = ["⌥", "⌥", "⌥"]
|
||||
static let minShortcutCount = 1
|
||||
static let maxShortcutCount = 9
|
||||
// Matches `defaultShortcuts` (3 hold slots: holdShortcut / holdShortcut2 / holdShortcut3).
|
||||
static var shortcutCount = 3
|
||||
|
||||
static func indexToName(_ baseName: String, _ index: Int) -> String {
|
||||
return baseName + (index == 0 ? "" : String(index + 1))
|
||||
|
||||
@@ -5,7 +5,10 @@ import ShortcutRecorder
|
||||
class CustomRecorderControl: RecorderControl {
|
||||
static let allowedModifiers = NSEvent.ModifierFlags(arrayLiteral: [.command, .control, .option, .shift])
|
||||
var clearable: Bool!
|
||||
var id: String!
|
||||
/// The preference key this recorder edits. Derived from `identifier` (set in `init` and re-aimed
|
||||
/// by `TriggerBinding.bind` as the recycled editor switches shortcuts) so it can never drift out
|
||||
/// of sync with the key the write path (`controlWasChanged`) and the conflict detector key off.
|
||||
var id: String { identifier!.rawValue }
|
||||
|
||||
convenience init(_ shortcutString: String, _ clearable: Bool, _ id: String) {
|
||||
self.init(Shortcut(keyEquivalent: shortcutString), clearable, id)
|
||||
@@ -15,7 +18,7 @@ class CustomRecorderControl: RecorderControl {
|
||||
self.init(frame: .zero)
|
||||
translatesAutoresizingMaskIntoConstraints = false
|
||||
self.clearable = clearable
|
||||
self.id = id
|
||||
identifier = NSUserInterfaceItemIdentifier(id)
|
||||
delegate = self
|
||||
allowsEscapeToCancelRecording = false
|
||||
allowsDeleteToClearShortcutAndEndRecording = false
|
||||
@@ -41,49 +44,39 @@ class CustomRecorderControl: RecorderControl {
|
||||
set(allowedModifierFlags: CustomRecorderControl.allowedModifiers.subtracting(restrictedModifiers), requiredModifierFlags: [], allowsEmptyModifierFlags: true)
|
||||
}
|
||||
|
||||
/// How to clear the conflicting shortcut when the user picks "Unassign and continue". Resolved
|
||||
/// from the id alone — arrow/vim are toggled off via their checkboxes; everything else is cleared
|
||||
/// through `Preferences` by id (so a shortcut that isn't on screen is handled correctly). The
|
||||
/// human-readable label shown in the dialog comes separately from `ControlsTab.conflictLabel`.
|
||||
private enum ShortcutConflict {
|
||||
case arrow
|
||||
case vim
|
||||
case regular(control: CustomRecorderControl, label: String)
|
||||
case unknown
|
||||
|
||||
var label: String {
|
||||
switch self {
|
||||
case .arrow: return "Arrow keys"
|
||||
case .vim: return "Vim keys"
|
||||
case .regular(_, let label): return label
|
||||
case .unknown: return "an unknown action"
|
||||
}
|
||||
}
|
||||
case regular(id: String)
|
||||
|
||||
static func classify(_ id: String) -> ShortcutConflict {
|
||||
if ["←", "→", "↑", "↓"].contains(id) { return .arrow }
|
||||
if id.starts(with: "vimCycle") { return .vim }
|
||||
if let existing = ControlsTab.shortcutControls[id] {
|
||||
return .regular(control: existing.0, label: existing.1)
|
||||
}
|
||||
assertionFailure("conflict id '\(id)' missing from shortcutControls")
|
||||
return .unknown
|
||||
return .regular(id: id)
|
||||
}
|
||||
}
|
||||
|
||||
func alertIfSameShortcutAlreadyAssigned(_ candidateShortcut: Shortcut, _ shortcutAlreadyAssigned: String) {
|
||||
let conflict = ShortcutConflict.classify(shortcutAlreadyAssigned)
|
||||
// `conflictLabel` returns nil only for an id with no known action, which can't happen for a
|
||||
// real detected conflict; keep the prior plain-string fallback (not a new l10n key).
|
||||
let label = ControlsTab.conflictLabel(shortcutAlreadyAssigned) ?? "an unknown action"
|
||||
let alert = NSAlert()
|
||||
alert.alertStyle = .warning
|
||||
alert.messageText = NSLocalizedString("Conflicting shortcut", comment: "")
|
||||
alert.informativeText = String(format: NSLocalizedString("Shortcut already assigned to another action: %@", comment: ""),
|
||||
conflict.label.replacingOccurrences(of: " ", with: "\u{00A0}"))
|
||||
if !id.starts(with: "holdShortcut") {
|
||||
alert.addButton(withTitle: NSLocalizedString("Unassign existing shortcut and continue", comment: "")).setAccessibilityFocused(true)
|
||||
}
|
||||
alert.informativeText = String(format: NSLocalizedString("Shortcut already assigned to: %@", comment: ""),
|
||||
label.replacingOccurrences(of: " ", with: "\u{00A0}"))
|
||||
// Always offer to resolve it, including when editing a Hold: unassigning a conflicting Trigger
|
||||
// clears its "and press" part (the hold itself is never the thing unassigned).
|
||||
alert.addButton(withTitle: NSLocalizedString("Unassign existing shortcut and continue", comment: "")).setAccessibilityFocused(true)
|
||||
let cancelButton = alert.addButton(withTitle: NSLocalizedString("Cancel", comment: ""))
|
||||
cancelButton.keyEquivalent = "\u{1b}"
|
||||
if id.starts(with: "holdShortcut") {
|
||||
cancelButton.setAccessibilityFocused(true)
|
||||
}
|
||||
let userChoice = alert.runModal()
|
||||
guard !id.starts(with: "holdShortcut"), userChoice == .alertFirstButtonReturn else { return }
|
||||
guard userChoice == .alertFirstButtonReturn else { return }
|
||||
switch conflict {
|
||||
case .arrow:
|
||||
if let cb = ControlsTab.arrowKeysCheckbox {
|
||||
@@ -97,10 +90,8 @@ class CustomRecorderControl: RecorderControl {
|
||||
ControlsTab.vimKeysEnabledCallback(cb)
|
||||
LabelAndControl.controlWasChanged(cb, nil)
|
||||
}
|
||||
case .regular(let existingControl, _):
|
||||
updateShortcut(existingControl, nil, existingControl, shortcutAlreadyAssigned)
|
||||
case .unknown:
|
||||
return
|
||||
case .regular(let conflictingId):
|
||||
ControlsTab.unassignShortcut(conflictingId)
|
||||
}
|
||||
updateShortcut(self, candidateShortcut, self, id)
|
||||
}
|
||||
@@ -112,23 +103,18 @@ class CustomRecorderControl: RecorderControl {
|
||||
}
|
||||
|
||||
func alertIfShortcutReservedByMacos(_ candidateShortcut: Shortcut, _ shortcutReservedByMacos: String) {
|
||||
let existing = ControlsTab.shortcutControls[shortcutReservedByMacos]
|
||||
if existing == nil {
|
||||
assertionFailure("reserved-by-macos id '\(shortcutReservedByMacos)' missing from shortcutControls")
|
||||
}
|
||||
let label = ControlsTab.conflictLabel(shortcutReservedByMacos) ?? "an unknown action"
|
||||
let alert = NSAlert()
|
||||
alert.alertStyle = .warning
|
||||
alert.messageText = NSLocalizedString("Conflicting shortcut", comment: "")
|
||||
alert.informativeText = String(format: NSLocalizedString("macOS reserves ⌘⌥⎋, ⌘⌥⇧⎋, and ⌘⌥⇧⌃⎋ for Force Quit and they cannot be unbound. AltTab cannot use them.\n\nYour change would assign one of these to: %@.", comment: ""), existing?.1 ?? "an unknown action")
|
||||
alert.informativeText = String(format: NSLocalizedString("macOS reserves ⌘⌥⎋, ⌘⌥⇧⎋, and ⌘⌥⇧⌃⎋ for Force Quit and they cannot be unbound. AltTab cannot use them.\n\nYour change would assign one of these to: %@.", comment: ""), label)
|
||||
alert.addButton(withTitle: NSLocalizedString("Unassign existing shortcut and continue", comment: ""))
|
||||
let cancelButton = alert.addButton(withTitle: NSLocalizedString("Cancel", comment: ""))
|
||||
cancelButton.keyEquivalent = "\u{1b}"
|
||||
cancelButton.setAccessibilityFocused(true)
|
||||
let userChoice = alert.runModal()
|
||||
guard userChoice == .alertFirstButtonReturn, id != shortcutReservedByMacos else { return }
|
||||
if let existing {
|
||||
updateShortcut(existing.0, nil, existing.0, shortcutReservedByMacos)
|
||||
}
|
||||
ControlsTab.unassignShortcut(shortcutReservedByMacos)
|
||||
updateShortcut(self, candidateShortcut, self, id)
|
||||
}
|
||||
|
||||
|
||||
@@ -18,6 +18,11 @@ existing AltTab shortcut or a macOS-reserved one.
|
||||
- A combo **reserved by macOS** is rejected.
|
||||
- Regression (#5585): a Cmd-based hold shortcut is **no longer blocked** by the macOS 26 Game Overlay
|
||||
reservation — that specific over-broad rejection was removed.
|
||||
- The `candidateId` handed in must be a **well-formed bound preference key** (`isWellFormedCandidateId`):
|
||||
a `holdShortcut`/`nextWindowShortcut` id has to resolve to an in-range shortcut index. The recycled
|
||||
`ShortcutEditor` regression fed a frozen placeholder (`"nextWindowShortcut0"`, index -1) that matched
|
||||
nothing and silently returned `.accepted`, so the conflict dialog stopped appearing. A `#if DEBUG`
|
||||
assert now trips loudly if a malformed id ever reaches the check.
|
||||
|
||||
## Test scenarios
|
||||
|
||||
@@ -27,9 +32,14 @@ Mirrors `CustomRecorderControlTests.swift` 1:1.
|
||||
- **testIsShortcutAcceptable_accepted** — a valid key+modifier combo is accepted.
|
||||
- **testIsShortcutAcceptable_modifiersOnlyButContainsKeycode** — modifiers-only flags but with a real keycode → accepted.
|
||||
- **testIsShortcutAcceptable_conflictWithExistingShortcut** — collides with an existing shortcut → rejected.
|
||||
- **testIsShortcutAcceptable_holdChangeStripsOldHoldFromCombinedNextWindow** — changing a shortcut's hold (e.g. ⌘→⌥) so its chord now duplicates another shortcut is detected; the same-index nextWindow is stored COMBINED with the old hold, so the old hold is stripped before applying the new one (else ⌥⌘+Tab is compared instead of ⌥+Tab and the conflict is missed).
|
||||
- **testIsShortcutAcceptable_reservedByMacos** — a macOS-reserved combo → rejected.
|
||||
- **testIsShortcutAcceptable_cmdHoldShortcutNoLongerBlockedByGameOverlay** — regression #5585: a Cmd hold shortcut is accepted (Game Overlay no longer blocks it).
|
||||
|
||||
### isWellFormedCandidateId
|
||||
Guards the recycled-`ShortcutEditor` regression where a stale recorder id (id/identifier drift) reached the conflict check and silently suppressed the dialog.
|
||||
- **testIsWellFormedCandidateId** — hold/next ids must resolve to an in-range index; the `"…Shortcut0"` placeholders (index -1) and out-of-range ids are rejected; static/arrow/vim ids are always well-formed.
|
||||
|
||||
### combinedModifiersMatch
|
||||
Used by the keyboard matcher to recognize a chord whose modifiers are physically split between the configured `holdShortcut` and a local shortcut (e.g. commandShiftTab = ⌘⌥-hold + ⇧).
|
||||
- **testCombinedModifiersMatchEqualToItself** — a modifier set matches itself (trivial union).
|
||||
|
||||
@@ -3,7 +3,19 @@ import Carbon.HIToolbox.Events
|
||||
import ShortcutRecorder
|
||||
|
||||
class CustomRecorderControlTestable {
|
||||
/// A `holdShortcut`/`nextWindowShortcut` candidate id must resolve to an in-range shortcut index;
|
||||
/// every other id (the static "when active" shortcuts, arrow, vim) is well-formed by definition.
|
||||
/// This is the contract the recycled `ShortcutEditor` must uphold: the id handed to the conflict
|
||||
/// check is the bound preference key, not a stale placeholder. The regression that hid the
|
||||
/// conflict dialog was exactly a malformed id — the recorder's frozen `"nextWindowShortcut0"`,
|
||||
/// whose `nameToIndex` is -1 — silently matching nothing and returning `.accepted`.
|
||||
static func isWellFormedCandidateId(_ id: String) -> Bool {
|
||||
guard id.hasPrefix("holdShortcut") || id.hasPrefix("nextWindowShortcut") else { return true }
|
||||
return (0..<Preferences.shortcutCount).contains(Preferences.nameToIndex(id))
|
||||
}
|
||||
|
||||
static func isShortcutAcceptable(_ candidateId: String, _ candidateShortcut: Shortcut) -> ShortcutAcceptance {
|
||||
assert(isWellFormedCandidateId(candidateId), "malformed candidateId '\(candidateId)' reached the conflict check — recycled editor id/identifier drift?")
|
||||
if let currentShortcutWithSameId = ControlsTab.shortcuts[candidateId]?.shortcut,
|
||||
candidateShortcut.carbonKeyCode == currentShortcutWithSameId.carbonKeyCode && candidateShortcut.carbonModifierFlags == currentShortcutWithSameId.carbonModifierFlags {
|
||||
return .accepted
|
||||
@@ -32,11 +44,16 @@ class CustomRecorderControlTestable {
|
||||
&& !(atShortcut.shortcut.keyCode == .none && atShortcut.shortcut.modifierFlags == []) else { continue }
|
||||
// candidate is holdShortcut
|
||||
if candidateId.starts(with: "holdShortcut") {
|
||||
// combine with local shortcut
|
||||
if (!atShortcut.id.starts(with: "holdShortcut") && !atShortcut.id.starts(with: "nextWindowShortcut"))
|
||||
// combine with nextWindowShortcut of same index
|
||||
|| (atShortcut.id.starts(with: "nextWindowShortcut") && Preferences.nameToIndex(candidateId) == Preferences.nameToIndex(atShortcut.id)) {
|
||||
// combine the candidate's (new) hold modifiers with a local shortcut, which is stored raw
|
||||
if !atShortcut.id.starts(with: "holdShortcut") && !atShortcut.id.starts(with: "nextWindowShortcut") {
|
||||
combos.append((atShortcut.id, Shortcut(code: atShortcut.shortcut.keyCode, modifierFlags: [candidateShortcut.modifierFlags, atShortcut.shortcut.modifierFlags], characters: nil, charactersIgnoringModifiers: nil)))
|
||||
// combine with the nextWindowShortcut of the same index. That shortcut is stored COMBINED
|
||||
// with the CURRENT (old) hold, so strip the old hold before applying the candidate's new
|
||||
// hold — otherwise changing the hold (e.g. ⌘→⌥) leaks the old modifier (⌥⌘⇥ instead of
|
||||
// ⌥⇥) and a real conflict is missed. (No-op when nextWindow is stored raw, e.g. in tests.)
|
||||
} else if atShortcut.id.starts(with: "nextWindowShortcut") && Preferences.nameToIndex(candidateId) == Preferences.nameToIndex(atShortcut.id) {
|
||||
let oldHoldModifiers = ControlsTab.shortcuts[Preferences.indexToName("holdShortcut", Preferences.nameToIndex(candidateId))]?.shortcut.modifierFlags ?? []
|
||||
combos.append((atShortcut.id, Shortcut(code: atShortcut.shortcut.keyCode, modifierFlags: [candidateShortcut.modifierFlags, atShortcut.shortcut.modifierFlags.subtracting(oldHoldModifiers)], characters: nil, charactersIgnoringModifiers: nil)))
|
||||
}
|
||||
// candidate is nextWindowShortcut
|
||||
} else if candidateId.starts(with: "nextWindowShortcut") {
|
||||
|
||||
@@ -27,6 +27,25 @@ final class CustomRecorderControlTests: XCTestCase {
|
||||
XCTAssertEqual(CustomRecorderControlTestable.isShortcutAcceptable("nextWindowShortcut", Shortcut(keyEquivalent: "⇧")!), .conflictWithExistingShortcut(shortcutAlreadyAssigned: "previousWindowShortcut"))
|
||||
}
|
||||
|
||||
/// Regression: S1 = ⌥+Tab, S2 = ⌘+Tab. Changing S2's *hold* from ⌘ to ⌥ makes it ⌥+Tab, which must
|
||||
/// conflict with S1. The registry here mirrors PRODUCTION, where `nextWindowShortcut` is stored
|
||||
/// COMBINED (hold ∪ key) — unlike `defaultShortcuts`, which stores it raw, which is precisely why
|
||||
/// the existing tests never exercised this. Without stripping S2's current ⌘ hold before applying
|
||||
/// the new ⌥, the chord is mis-computed as ⌥⌘+Tab and the conflict is silently accepted.
|
||||
func testIsShortcutAcceptable_holdChangeStripsOldHoldFromCombinedNextWindow() {
|
||||
ControlsTab.shortcuts = [
|
||||
"holdShortcut": ATShortcut(Shortcut(keyEquivalent: "⌥")!, "holdShortcut", .global, .up, 0),
|
||||
"nextWindowShortcut": ATShortcut(Shortcut(keyEquivalent: "⌥⇥")!, "nextWindowShortcut", .global, .down),
|
||||
"holdShortcut2": ATShortcut(Shortcut(keyEquivalent: "⌘")!, "holdShortcut2", .global, .up, 1),
|
||||
"nextWindowShortcut2": ATShortcut(Shortcut(keyEquivalent: "⌘⇥")!, "nextWindowShortcut2", .global, .down),
|
||||
]
|
||||
defer { ControlsTab.shortcuts = ControlsTab.defaultShortcuts }
|
||||
XCTAssertEqual(CustomRecorderControlTestable.isShortcutAcceptable("holdShortcut2", Shortcut(keyEquivalent: "⌥")!),
|
||||
.conflictWithExistingShortcut(shortcutAlreadyAssigned: "nextWindowShortcut"))
|
||||
// The mirror case: changing the hold to something that does NOT collide stays accepted.
|
||||
XCTAssertEqual(CustomRecorderControlTestable.isShortcutAcceptable("holdShortcut2", Shortcut(keyEquivalent: "⌃")!), .accepted)
|
||||
}
|
||||
|
||||
func testIsShortcutAcceptable_reservedByMacos() {
|
||||
XCTAssertEqual(CustomRecorderControlTestable.isShortcutAcceptable("previousWindowShortcut", Shortcut(keyEquivalent: "⌘⇧")!), .accepted) // ⌘⎋
|
||||
XCTAssertEqual(CustomRecorderControlTestable.isShortcutAcceptable("previousWindowShortcut", Shortcut(keyEquivalent: "⌘⌃⇧")!), .accepted) // ⌘⎋
|
||||
@@ -74,6 +93,30 @@ final class CustomRecorderControlTests: XCTestCase {
|
||||
XCTAssertEqual(CustomRecorderControlTestable.isShortcutAcceptable("holdShortcut", Shortcut(keyEquivalent: "⌘")!), .accepted)
|
||||
}
|
||||
|
||||
// MARK: - isWellFormedCandidateId
|
||||
|
||||
/// Regression guard for the conflict dialog that silently stopped appearing under the recycled
|
||||
/// `ShortcutEditor`: the recorder handed the kernel a frozen placeholder id (`"nextWindowShortcut0"`,
|
||||
/// whose `nameToIndex` is -1), which matched no shortcut, so `isShortcutAcceptable` returned
|
||||
/// `.accepted` and no dialog showed. `isWellFormedCandidateId` rejects exactly that shape — and a
|
||||
/// `#if DEBUG` assert in `isShortcutAcceptable` now trips loudly if such an id ever reaches it.
|
||||
func testIsWellFormedCandidateId() {
|
||||
// hold/next ids must resolve to an in-range shortcut index (shortcutCount == 3 in the mock).
|
||||
XCTAssertTrue(CustomRecorderControlTestable.isWellFormedCandidateId("holdShortcut")) // index 0
|
||||
XCTAssertTrue(CustomRecorderControlTestable.isWellFormedCandidateId("nextWindowShortcut")) // index 0
|
||||
XCTAssertTrue(CustomRecorderControlTestable.isWellFormedCandidateId("nextWindowShortcut2")) // index 1
|
||||
XCTAssertTrue(CustomRecorderControlTestable.isWellFormedCandidateId("holdShortcut3")) // index 2
|
||||
// The regressing placeholders: trailing "0" → nameToIndex -1 → out of range.
|
||||
XCTAssertFalse(CustomRecorderControlTestable.isWellFormedCandidateId("nextWindowShortcut0"))
|
||||
XCTAssertFalse(CustomRecorderControlTestable.isWellFormedCandidateId("holdShortcut0"))
|
||||
// Beyond shortcutCount is also malformed.
|
||||
XCTAssertFalse(CustomRecorderControlTestable.isWellFormedCandidateId("nextWindowShortcut4")) // index 3
|
||||
// Static "when active" / arrow / vim ids are well-formed regardless of any index.
|
||||
XCTAssertTrue(CustomRecorderControlTestable.isWellFormedCandidateId("cancelShortcut"))
|
||||
XCTAssertTrue(CustomRecorderControlTestable.isWellFormedCandidateId("←"))
|
||||
XCTAssertTrue(CustomRecorderControlTestable.isWellFormedCandidateId("vimCycleLeft"))
|
||||
}
|
||||
|
||||
// MARK: - combinedModifiersMatch
|
||||
|
||||
// Per the kernel's doc-comment: the function checks whether two modifier sets match once each is
|
||||
|
||||
@@ -52,6 +52,21 @@ class ControlsTab {
|
||||
"focusWindowShortcut", "previousWindowShortcut", "cancelShortcut", "searchShortcut", "lockSearchShortcut",
|
||||
"closeWindowShortcut", "minDeminWindowShortcut", "toggleFullscreenWindowShortcut", "quitAppShortcut", "hideShowAppShortcut",
|
||||
]
|
||||
/// Canonical id → localized label for the always-active ("when active") shortcuts. Single source
|
||||
/// of truth: `ShortcutsWhenActiveSheet` reads its row titles from here, and `conflictLabel(_:)`
|
||||
/// uses it to name a conflicting shortcut without needing its (possibly-unbuilt) sheet control.
|
||||
static let staticShortcutLabels = [
|
||||
"focusWindowShortcut": NSLocalizedString("Focus selected window", comment: ""),
|
||||
"previousWindowShortcut": NSLocalizedString("Select previous window", comment: ""),
|
||||
"cancelShortcut": NSLocalizedString("Cancel", comment: ""),
|
||||
"searchShortcut": NSLocalizedString("Search", comment: ""),
|
||||
"lockSearchShortcut": NSLocalizedString("Lock search", comment: ""),
|
||||
"closeWindowShortcut": NSLocalizedString("Close window", comment: ""),
|
||||
"minDeminWindowShortcut": NSLocalizedString("Minimize/Deminimize window", comment: ""),
|
||||
"toggleFullscreenWindowShortcut": NSLocalizedString("Fullscreen/Defullscreen window", comment: ""),
|
||||
"quitAppShortcut": NSLocalizedString("Quit app", comment: ""),
|
||||
"hideShowAppShortcut": NSLocalizedString("Hide/Show app", comment: ""),
|
||||
]
|
||||
private static let removableShortcutPreferences = [
|
||||
"holdShortcut", "nextWindowShortcut",
|
||||
"appsToShow", "spacesToShow", "screensToShow",
|
||||
@@ -916,17 +931,37 @@ class ControlsTab {
|
||||
return true
|
||||
}
|
||||
|
||||
private static func conflictLabel(_ controlId: String) -> String? {
|
||||
if let shortcutControl = shortcutControls[controlId] {
|
||||
return shortcutControl.1
|
||||
/// Human-readable label for the action bound to `id`, resolved purely from the model — the id's
|
||||
/// shape plus `staticShortcutLabels` — NOT from `shortcutControls`. This is what lets the conflict
|
||||
/// dialog name a shortcut that isn't currently displayed in the recycled editor (which keeps only
|
||||
/// the on-screen shortcut in `shortcutControls`). Returns nil for ids with no known action.
|
||||
///
|
||||
/// A numbered shortcut's hold and "and press" both belong to that shortcut's Trigger, so they
|
||||
/// resolve to e.g. "Shortcut 2 - Trigger" — naming WHICH shortcut, since "Select next window"
|
||||
/// alone doesn't disambiguate when several shortcuts exist.
|
||||
static func conflictLabel(_ id: String) -> String? {
|
||||
if arrowKeys.contains(id) { return NSLocalizedString("Arrow keys", comment: "") }
|
||||
if vimKeyActions.values.contains(id) { return NSLocalizedString("Vim keys", comment: "") }
|
||||
if id.hasPrefix("holdShortcut") || id.hasPrefix("nextWindowShortcut") {
|
||||
return shortcutTitle(Preferences.nameToIndex(id)) + " - " + ShortcutEditor.triggerLabel
|
||||
}
|
||||
if arrowKeys.contains(controlId) {
|
||||
return NSLocalizedString("Arrow keys", comment: "")
|
||||
}
|
||||
if vimKeyActions.values.contains(controlId) {
|
||||
return NSLocalizedString("Vim keys", comment: "")
|
||||
}
|
||||
return nil
|
||||
return staticShortcutLabels[id]
|
||||
}
|
||||
|
||||
/// Clear the shortcut bound to `id` and let the normal preference-change pipeline reconcile the
|
||||
/// registry and UI. Used by the conflict dialog's "Unassign existing shortcut and continue" for a
|
||||
/// shortcut that may not be on screen, so it goes through `Preferences` (cached UserDefaults)
|
||||
/// rather than mutating a live recorder. If that recorder happens to be displayed, sync it too.
|
||||
///
|
||||
/// For a numbered shortcut's Trigger the hold can't stand alone, so "unassign" clears the "and
|
||||
/// press" (nextWindowShortcut) part — e.g. ⌥+Tab becomes ⌥+(unassigned) — whether the conflict
|
||||
/// was reported against the hold or the press.
|
||||
static func unassignShortcut(_ id: String) {
|
||||
let keyToClear = (id.hasPrefix("holdShortcut") || id.hasPrefix("nextWindowShortcut"))
|
||||
? Preferences.indexToName("nextWindowShortcut", Preferences.nameToIndex(id))
|
||||
: id
|
||||
Preferences.setShortcut(keyToClear, nil)
|
||||
shortcutControls[keyToClear]?.0.objectValue = nil
|
||||
}
|
||||
|
||||
private static func shouldClearConflictingShortcuts(_ conflicts: [String], _ messageFormat: String) -> Bool {
|
||||
|
||||
@@ -220,8 +220,7 @@ final class TriggerBinding {
|
||||
private let andPressLabel: NSTextField
|
||||
private static let holdLabelText = NSLocalizedString("Hold", comment: "")
|
||||
private static let andPressText = NSLocalizedString("and press", comment: "")
|
||||
// Not shown as a trigger-row label anymore; kept as the next-window recorder's human-readable
|
||||
// name in `shortcutControls`, used by ControlsTab's conflict dialogs (see `conflictLabel`).
|
||||
// The next-window recorder's human-readable name, stored alongside it in `shortcutControls`.
|
||||
private static let selectNextText = NSLocalizedString("Select next window", comment: "")
|
||||
private var currentIndex: Int = -1
|
||||
|
||||
|
||||
@@ -2,16 +2,18 @@ import Cocoa
|
||||
|
||||
class ShortcutsWhenActiveSheet: SheetWindow {
|
||||
private static let title = NSLocalizedString("Shortcuts When Active", comment: "")
|
||||
private static let labelFocus = NSLocalizedString("Focus selected window", comment: "")
|
||||
private static let labelPrevious = NSLocalizedString("Select previous window", comment: "")
|
||||
private static let labelCancel = NSLocalizedString("Cancel", comment: "")
|
||||
private static let labelSearch = NSLocalizedString("Search", comment: "")
|
||||
private static let labelLockSearch = NSLocalizedString("Lock search", comment: "")
|
||||
private static let labelClose = NSLocalizedString("Close window", comment: "")
|
||||
private static let labelMinDemin = NSLocalizedString("Minimize/Deminimize window", comment: "")
|
||||
private static let labelFullscreen = NSLocalizedString("Fullscreen/Defullscreen window", comment: "")
|
||||
private static let labelQuit = NSLocalizedString("Quit app", comment: "")
|
||||
private static let labelHideShow = NSLocalizedString("Hide/Show app", comment: "")
|
||||
// Row titles come from `ControlsTab.staticShortcutLabels` (single source of truth, also used by
|
||||
// the conflict dialog to name these actions).
|
||||
private static let labelFocus = ControlsTab.staticShortcutLabels["focusWindowShortcut"]!
|
||||
private static let labelPrevious = ControlsTab.staticShortcutLabels["previousWindowShortcut"]!
|
||||
private static let labelCancel = ControlsTab.staticShortcutLabels["cancelShortcut"]!
|
||||
private static let labelSearch = ControlsTab.staticShortcutLabels["searchShortcut"]!
|
||||
private static let labelLockSearch = ControlsTab.staticShortcutLabels["lockSearchShortcut"]!
|
||||
private static let labelClose = ControlsTab.staticShortcutLabels["closeWindowShortcut"]!
|
||||
private static let labelMinDemin = ControlsTab.staticShortcutLabels["minDeminWindowShortcut"]!
|
||||
private static let labelFullscreen = ControlsTab.staticShortcutLabels["toggleFullscreenWindowShortcut"]!
|
||||
private static let labelQuit = ControlsTab.staticShortcutLabels["quitAppShortcut"]!
|
||||
private static let labelHideShow = ControlsTab.staticShortcutLabels["hideShowAppShortcut"]!
|
||||
|
||||
/// Pre-build search index for the open-button. See `SettingsSearchIndex.sheetSearchableStrings`.
|
||||
/// `ProBadgeView.proLabel` contributes the "Pro" tag rendered on the search/lock-search rows.
|
||||
|
||||
Reference in new issue
Block a user