From c6a50f511b56aceab0be608cd5d31e995615c55c Mon Sep 17 00:00:00 2001 From: Richard Boisvert Date: Sat, 5 Sep 2026 16:45:24 -0400 Subject: [PATCH 1/2] feat(menu-bar): change the shortcut modifier from the menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Right-click the icon and Shortcut Modifier offers ⌃⌘, ⌥⌘, ⇧⌘, ⌃⌥ and ⌃⌥⌘, with the one in force ticked. It changes the modifier every shortcut uses and keeps each key: which key does what is a matter of taste that `defaults write` already serves, while the modifier is the part that collides with other software, and so the part worth being able to change without a restart. The icon flashes green when a change takes and red when it does not, in which case nothing is stored and the log names the combination and the app that already holds it. The colour is drawn into the image rather than tinted over it. A template image is a mask, so a colour applied to one goes nowhere -- a status item button ignores `contentTintColor` entirely, which was measured against the running app rather than assumed. All three move together or none does. The new combinations are claimed before the old ones are released, so discovering that one of them belongs to another app costs nothing: the partial set is dropped, the working shortcuts were never given up, and the log says only what is true. Announcing a registration is therefore separate from making it -- claiming a set that may be abandoned a moment later must not put combinations in the log that were unregistered before anyone could press one. Two things a lighter version of this would get wrong. Whether the choice changes anything is decided by comparing combinations and not the text of them: what is stored is however it was written, "cmd+ctrl+h", and what a rewrite produces is canonical, "ctrl+cmd+h", so the very same chord reads as a change -- and claiming a combination this process already holds is refused, which would report a failure for the one case that cannot fail. And the choice is stored only if the configuration it was made against is still the one in force, because it is read on the agent's queue and applied on the main one; a reload arriving in between is the newer intent, and its shortcuts are put back rather than overwritten. ⇧⌘ and ⌥⌘ are offered with a tooltip saying what they cost -- selecting a line, and moving between browser tabs -- and neither is the default. Heed can only refuse a combination another app registered the same way; nothing refuses one the system reads directly, so those two are taken quietly and a warning before the fact is the only one there is. --- README.md | 18 +- Sources/FFMCore/HotkeySpec.swift | 99 +++++++++++ Sources/Heed/Agent.swift | 216 +++++++++++++++++++---- Sources/Heed/MenuBar.swift | 79 ++++++++- Tests/FFMCoreTests/HotkeySpecTests.swift | 85 +++++++++ 5 files changed, 452 insertions(+), 45 deletions(-) diff --git a/README.md b/README.md index 7837b78..acf6d36 100644 --- a/README.md +++ b/README.md @@ -44,8 +44,9 @@ make uninstall Click the cube in the menu bar to turn focus following on or off. A dim icon means Heed is off or does not have Accessibility permission. Hover over it to see which one. -Right-click or control-click the icon to open the log, see the installed version, or quit. Quitting -unloads the current login agent. Heed starts again at the next login. +Right-click or control-click the icon to change the shortcut modifier, open the log, see the +installed version, or quit. Quitting unloads the current login agent. Heed starts again at the next +login. Press **Control+Command+H** to toggle Heed from anywhere. This is often easier than moving the pointer to the menu bar. @@ -67,7 +68,7 @@ Change or disable any of them with: ```sh defaults write io.github.rbstp.heed hotkey 'cmd+ctrl+alt+f' -defaults write io.github.rbstp.heed focusNextHotkey 'cmd+shift+right' +defaults write io.github.rbstp.heed focusNextHotkey 'cmd+ctrl+alt+right' defaults write io.github.rbstp.heed hotkey '' make restart ``` @@ -75,9 +76,18 @@ make restart A hotkey needs at least one modifier other than Shift. If another app already owns it, Heed refuses to register it and writes the reason to the log. +**Shortcut Modifier** in the right-click menu changes the modifier all three shortcuts use, keeping +each key, without a restart. The icon flashes green when the change takes and red when it does not, +in which case nothing is changed and the log names the combination and the app that already holds +it. Every shortcut moves together or none does, so they cannot end up under two different modifiers. + Heed registers a hotkey exclusively, which takes the combination away from every other app. That is why the focus shortcuts default to Control and not Shift: Command+Shift+Left and Command+Shift+Right -are how a line is selected in every text field on the system. +are how a line is selected in every text field on the system, and Command+Option+Left and +Command+Option+Right move between tabs in most browsers and terminals. Both are offered in the menu +anyway, with a tooltip saying what they cost — but neither is the default. Heed can only refuse a +combination another app registered the same way; nothing refuses one the system reads directly, so +those two are taken quietly. To hide the menu bar icon: diff --git a/Sources/FFMCore/HotkeySpec.swift b/Sources/FFMCore/HotkeySpec.swift index 2f544fc..63c8f39 100644 --- a/Sources/FFMCore/HotkeySpec.swift +++ b/Sources/FFMCore/HotkeySpec.swift @@ -54,6 +54,26 @@ public struct HotkeySpec: Equatable, Sendable { keyCode = code } + /// The same key under different modifiers, or nil when that would not be a legal hotkey. + /// + /// Goes back through the parser rather than building a value directly, so a combination arrived + /// at by changing the modifiers has to pass exactly the same rules as one someone typed -- + /// including the one that matters, that shift alone is not a chord. + public func withModifiers(_ modifiers: Set) -> HotkeySpec? { + HotkeySpec((modifiers.map(\.rawValue) + [key]).joined(separator: "+")) + } + + /// The combination in the form `defaults write` takes, which is the form this parses. The + /// counterpart to `display`: that one is for reading, this one is for storing. + public var written: String { + var parts: [String] = [] + if modifiers.contains(.control) { parts.append("ctrl") } + if modifiers.contains(.option) { parts.append("alt") } + if modifiers.contains(.shift) { parts.append("shift") } + if modifiers.contains(.command) { parts.append("cmd") } + return (parts + [key]).joined(separator: "+") + } + /// The combination the way macOS writes it, in the order macOS orders it: ⌃⌥⇧⌘ then the key. /// Used in the log and anywhere the hotkey has to be shown. public var display: String { @@ -93,3 +113,82 @@ public struct HotkeySpec: Equatable, Sendable { "semicolon": 41, "quote": 39, "backslash": 42, "leftbracket": 33, "rightbracket": 30, ] } + +/// Rewrite a hotkey setting under different modifiers, keeping its key. +/// +/// A setting that names no hotkey -- empty, or "none" -- is left exactly as it is. There is no key +/// to keep, and a shortcut somebody switched off must not come back because they changed the +/// modifier. So is anything that does not parse, which was already reported when it was loaded. +public func rewriteHotkey(_ text: String, modifiers: Set) -> String { + let wanted = text.trimmingCharacters(in: .whitespaces) + guard !wanted.isEmpty, wanted.lowercased() != "none", + let spec = HotkeySpec(wanted), + let changed = spec.withModifiers(modifiers) + else { return text } + return changed.written +} + +/// The modifier combinations the menu offers, in the order it offers them. +/// +/// A short list rather than every legal combination: this is a menu, and the point of it is to +/// change the modifier without going near `defaults write`, which remains there for anyone who +/// wants something else. +public enum ModifierPreset: CaseIterable, Sendable { + case controlCommand, optionCommand, shiftCommand, controlOption, controlOptionCommand + + public var modifiers: Set { + switch self { + case .controlCommand: [.control, .command] + case .optionCommand: [.option, .command] + case .shiftCommand: [.shift, .command] + case .controlOption: [.control, .option] + case .controlOptionCommand: [.control, .option, .command] + } + } + + /// The way macOS writes it, which is what the menu shows. + public var display: String { + var text = "" + if modifiers.contains(.control) { text += "⌃" } + if modifiers.contains(.option) { text += "⌥" } + if modifiers.contains(.shift) { text += "⇧" } + if modifiers.contains(.command) { text += "⌘" } + return text + } + + /// In words, for the tooltip: the symbols are how a Mac names these, but they are not how + /// anyone would say one out loud. + public var spoken: String { + var parts: [String] = [] + if modifiers.contains(.control) { parts.append("Control") } + if modifiers.contains(.option) { parts.append("Option") } + if modifiers.contains(.shift) { parts.append("Shift") } + if modifiers.contains(.command) { parts.append("Command") } + return parts.joined(separator: "-") + } + + /// What is already spoken for, or nil when there is nothing to say. + /// + /// Registering a hotkey takes the combination away from every other app, and Carbon only + /// refuses one that *another app* claimed the same way. Nothing refuses a combination the + /// system reads directly, so these two are taken quietly and the only warning possible is this + /// one, before the fact. + public var caution: String? { + switch self { + case .shiftCommand: + "Command-Shift with the arrow keys selects to the start or end of a line in every text " + + "field. Heed would take that away." + case .optionCommand: + "Command-Option with the arrow keys moves between tabs in most browsers and terminals. " + + "Heed would take that away." + default: nil + } + } + + /// Which of these a set of modifiers is, or nil when it is something else entirely -- somebody + /// typed their own into `defaults write`, and none of the offered ones should look chosen. + public static func matching(_ modifiers: Set?) -> ModifierPreset? { + guard let modifiers else { return nil } + return allCases.first { $0.modifiers == modifiers } + } +} diff --git a/Sources/Heed/Agent.swift b/Sources/Heed/Agent.swift index df9b554..5cfe363 100644 --- a/Sources/Heed/Agent.swift +++ b/Sources/Heed/Agent.swift @@ -254,7 +254,12 @@ final class Agent { onClick: { [weak self] in self?.toggleEnabled() }, // Not the agent's to do: quitting is about the process and the job launchd // holds it in, neither of which is state on `queue`. - onQuit: { quitHeed() } + onQuit: { quitHeed() }, + onChooseModifier: { [weak self] preset in + self?.changeModifiers(to: preset) { accepted in + self?.menuBar?.flash(accepted: accepted) + } + } ) } // Set here as well as in syncHotkey, because either can run first. @@ -265,56 +270,193 @@ final class Agent { } } + /// The shortcuts Heed registers. + /// + /// One list, so registering them, changing their modifier and storing them cannot come to + /// disagree about what exists. + private enum Shortcut: CaseIterable { + case toggle, focusNext, focusPrevious + + /// A third-person verb phrase, so it reads in both the line that says a hotkey works and + /// the line that says nothing does. + var which: String { + switch self { + case .toggle: "toggles Heed" + case .focusNext: "moves focus to the next window" + case .focusPrevious: "moves focus to the previous window" + } + } + + var defaultsKey: String { + switch self { + case .toggle: "hotkey" + case .focusNext: "focusNextHotkey" + case .focusPrevious: "focusPreviousHotkey" + } + } + + func text(in config: Config) -> String { + switch self { + case .toggle: config.hotkey + case .focusNext: config.focusNextHotkey + case .focusPrevious: config.focusPreviousHotkey + } + } + } + + private func action(for shortcut: Shortcut) -> () -> Void { + switch shortcut { + case .toggle: { [weak self] in self?.toggleEnabled() } + case .focusNext: { [weak self] in self?.stepFocus(by: 1) } + case .focusPrevious: { [weak self] in self?.stepFocus(by: -1) } + } + } + /// Registers every hotkey, replacing any previous one. Called on `queue`; Carbon registration /// belongs to the main thread, so the values are read here and applied there. private func syncHotkey() { - let toggle = config.hotkey - let next = config.focusNextHotkey - let previous = config.focusPreviousHotkey + let texts = Shortcut.allCases.map { $0.text(in: config) } DispatchQueue.main.async { [self] in // Dropping the old ones unregisters them, which is also how a changed combination takes // effect: there is no editing a registration in place. hotkeys = [] - shortcut = nil - menuBar?.shortcut = nil - - // Only the toggle reaches the menu: it is the one a menu can draw, and the one worth - // discovering there. See the key-equivalent note in MenuBarController. - shortcut = register(toggle, which: "toggles Heed", action: { [weak self] in - self?.toggleEnabled() - }) - menuBar?.shortcut = shortcut + adopt(specs: []) + + // Whatever can be had, rather than all or nothing: a combination another app holds + // should cost that one shortcut and not the other two. Changing the modifier is the + // opposite case, and says so there. + let claimed = claim(texts) + hotkeys = claimed.held + adopt(specs: claimed.specs) + announce(specs: claimed.specs) + } + } - register(next, which: "moves focus to the next window", action: { [weak self] in - self?.stepFocus(by: 1) - }) - register(previous, which: "moves focus to the previous window", action: { [weak self] in - self?.stepFocus(by: -1) - }) + /// Claim a set of combinations *without* giving up whatever is registered now. + /// + /// The registrations are returned rather than stored, and dropping one unregisters it. That is + /// what lets a change be tried before the working shortcuts are given up: Carbon refuses a + /// combination another app already holds, and finding that out must not cost the ones that + /// were fine. + /// + /// `specs` is one entry per shortcut, in `Shortcut.allCases` order, nil where nothing was + /// registered. `refused` is only about combinations that are taken -- a setting that is empty + /// or does not parse asked for nothing, so there was nothing to refuse. + private func claim(_ texts: [String]) -> (held: [Hotkey], specs: [HotkeySpec?], refused: Bool) { + dispatchPrecondition(condition: .onQueue(.main)) + var held: [Hotkey] = [] + var specs: [HotkeySpec?] = [] + var refused = false + + for (shortcut, text) in zip(Shortcut.allCases, texts) { + let wanted = text.trimmingCharacters(in: .whitespaces) + guard !wanted.isEmpty, wanted.lowercased() != "none" else { + specs.append(nil) + continue + } + guard let spec = HotkeySpec(wanted) else { + Log.note("hotkey \"\(wanted)\" is not a combination I understand " + + "(try cmd+ctrl+h); nothing \(shortcut.which)") + specs.append(nil) + continue + } + guard let registered = Hotkey(spec: spec, action: action(for: shortcut)) else { + specs.append(nil) + refused = true // Hotkey logs why + continue + } + held.append(registered) + specs.append(spec) } + return (held, specs, refused) } - /// Parse one combination and hold the registration. Main thread, called only from `syncHotkey`. + /// Say what is registered, once it is going to be kept. /// - /// `which` is a third-person verb phrase, so it reads in both the line that says a hotkey works - /// and the line that says nothing does. - @discardableResult - private func register( - _ text: String, which: String, action: @escaping () -> Void - ) -> HotkeySpec? { - let wanted = text.trimmingCharacters(in: .whitespaces) - guard !wanted.isEmpty, wanted.lowercased() != "none" else { return nil } - guard let spec = HotkeySpec(wanted) else { - Log.note("hotkey \"\(wanted)\" is not a combination I understand " - + "(try cmd+ctrl+h); nothing \(which)") - return nil + /// Not from inside `claim`, which is also used to *try* a set that may be given up a moment + /// later: announcing those would put two combinations in the log that were unregistered before + /// anyone could press them. + private func announce(specs: [HotkeySpec?]) { + for (shortcut, spec) in zip(Shortcut.allCases, specs) { + guard let spec else { continue } + Log.note("hotkey \(spec.display) \(shortcut.which)") } - guard let registered = Hotkey(spec: spec, action: action) else { - return nil // Hotkey logs why + } + + /// Tell the menu bar what is registered. Main thread; `specs` is in `Shortcut.allCases` order. + private func adopt(specs: [HotkeySpec?]) { + shortcut = specs.first.flatMap { $0 } + menuBar?.shortcut = shortcut + // From whichever shortcut actually has one, so the menu still shows the modifier in force + // when the toggle in particular has been switched off. + menuBar?.modifiers = specs.compactMap { $0 }.first?.modifiers + } + + /// Put every shortcut under a different modifier, keeping each one's key. + /// + /// All of them or none, and only stored if it took. Half-applying a modifier would leave the + /// menu showing one combination while another was registered, and the shortcuts under two + /// different modifiers is not a state anybody asked for. + /// + /// `report` is called on the main thread, with whether it took, so the menu bar can say so. + func changeModifiers(to preset: ModifierPreset, report: @escaping (Bool) -> Void) { + queue.async { [self] in + let current = Shortcut.allCases.map { $0.text(in: config) } + let texts = current.map { rewriteHotkey($0, modifiers: preset.modifiers) } + + // Compared as combinations and not as text. What is stored is however somebody wrote it + // -- "cmd+ctrl+h" -- and what comes back from `rewriteHotkey` is canonical -- + // "ctrl+cmd+h" -- so the very same chord reads as a change if the strings are compared. + // And claiming a combination this process already holds is refused by Carbon, which + // would report a failure for the one case that cannot fail: choosing the modifier that + // is already ticked. + guard !zip(current, texts).allSatisfy({ HotkeySpec($0) == HotkeySpec($1) }) else { + DispatchQueue.main.async { report(true) } + return + } + + DispatchQueue.main.async { [self] in + let claimed = claim(texts) + guard !claimed.refused else { + // `claimed.held` goes out of scope here, which unregisters the partial set. The + // working ones were never released, so there is nothing to put back. + Log.note("keeping the current shortcuts: \(preset.display) is not free") + report(false) + return + } + hotkeys = claimed.held // releases the old ones + adopt(specs: claimed.specs) + announce(specs: claimed.specs) + + queue.async { [self] in + // The configuration is read on this queue and applied on the main one, so it + // can have moved on in between -- a SIGHUP reload is the way that happens. Its + // reading is the newer one, and storing a modifier chosen against a + // configuration that is already gone would leave what is registered and what is + // stored describing different shortcuts until the next restart. Put the + // registrations back in step with it instead, and say the change did not take. + guard Shortcut.allCases.map({ $0.text(in: config) }) == current else { + Log.note("the configuration changed while \(preset.display) was being " + + "applied; keeping what it says instead") + syncHotkey() + DispatchQueue.main.async { report(false) } + return + } + + let store = Config.store() + for (shortcut, text) in zip(Shortcut.allCases, texts) { + store.set(text, forKey: shortcut.defaultsKey) + } + config.hotkey = texts[0] + config.focusNextHotkey = texts[1] + config.focusPreviousHotkey = texts[2] + Log.note("shortcuts now use \(preset.display)") + // Reported only once it is stored, so the icon is answering for the change that + // actually happened rather than for the registration alone. + DispatchQueue.main.async { report(true) } + } + } } - hotkeys.append(registered) - Log.note("hotkey \(spec.display) \(which)") - return spec } // MARK: - Main loop diff --git a/Sources/Heed/MenuBar.swift b/Sources/Heed/MenuBar.swift index 403317c..ff2ee37 100644 --- a/Sources/Heed/MenuBar.swift +++ b/Sources/Heed/MenuBar.swift @@ -10,15 +10,27 @@ final class MenuBarController: NSObject { private let item: NSStatusItem private let onClick: () -> Void private let onQuit: () -> Void + private let onChooseModifier: (ModifierPreset) -> Void private var state = MenuBarState(enabled: true, trusted: true) /// The registered hotkey, shown beside the toggle so the menu is where you find out it exists. /// Nil when none is registered. var shortcut: HotkeySpec? + /// The modifier every shortcut is registered under, so the menu can show which one is in force. + /// Nil when nothing is registered, or when it is a combination nobody offered. + var modifiers: Set? + /// The pending restore after a flash, so a second answer replaces the first rather than being + /// wiped by the first one's timer. + private var flashRestore: DispatchWorkItem? - init(onClick: @escaping () -> Void, onQuit: @escaping () -> Void) { + init( + onClick: @escaping () -> Void, + onQuit: @escaping () -> Void, + onChooseModifier: @escaping (ModifierPreset) -> Void + ) { dispatchPrecondition(condition: .onQueue(.main)) self.onClick = onClick self.onQuit = onQuit + self.onChooseModifier = onChooseModifier item = NSStatusBar.system.statusItem(withLength: NSStatusItem.squareLength) super.init() @@ -98,6 +110,10 @@ final class MenuBarController: NSObject { } menu.addItem(toggle) + let modifier = NSMenuItem(title: "Shortcut Modifier", action: nil, keyEquivalent: "") + modifier.submenu = modifierMenu() + menu.addItem(modifier) + let log = NSMenuItem(title: "Open Log", action: #selector(openLog), keyEquivalent: "") log.target = self menu.addItem(log) @@ -117,6 +133,58 @@ final class MenuBarController: NSObject { return menu } + /// The modifier every shortcut runs under, changed in one place rather than three. + /// + /// The keys are left alone: which key does what is a matter of taste that `defaults write` + /// already serves, while the modifier is the part that collides with other software and so the + /// part worth being able to change without a restart. + private func modifierMenu() -> NSMenu { + let menu = NSMenu() + let current = ModifierPreset.matching(modifiers) + for (index, preset) in ModifierPreset.allCases.enumerated() { + let item = NSMenuItem(title: preset.display, action: #selector(chooseModifier(_:)), + keyEquivalent: "") + item.target = self + item.tag = index + item.state = preset == current ? .on : .off + // The symbols are how a Mac names these and how VoiceOver reads them; the words and the + // warning are what a tooltip is for. + item.toolTip = [preset.spoken, preset.caution].compactMap { $0 }.joined(separator: ". ") + menu.addItem(item) + } + return menu + } + + @objc private func chooseModifier(_ sender: NSMenuItem) { + let presets = ModifierPreset.allCases + guard presets.indices.contains(sender.tag) else { return } + onChooseModifier(presets[sender.tag]) + } + + /// Say whether a change took, in the one place the user is already looking. + /// + /// The icon they just clicked rather than a notification somewhere else on screen. Refusal is + /// shown for longer than acceptance, because it is the one that asks you to do something about + /// it -- and the log names the combination and the app that already holds it. + /// + /// The colour is drawn into the image rather than tinted onto it. A template image is a *mask*, + /// so a colour applied over one goes nowhere: `contentTintColor` on the status item's button is + /// simply ignored, which was measured rather than assumed. + func flash(accepted: Bool) { + dispatchPrecondition(condition: .onQueue(.main)) + guard let button = item.button else { return } + + flashRestore?.cancel() + button.image = MenuBarController.icon(accepted ? .systemGreen : .systemRed) + + let restore = DispatchWorkItem { [weak self] in + self?.item.button?.image = MenuBarController.icon() + self?.flashRestore = nil + } + flashRestore = restore + DispatchQueue.main.asyncAfter(deadline: .now() + (accepted ? 0.7 : 1.3), execute: restore) + } + private static func modifierMask(_ spec: HotkeySpec) -> NSEvent.ModifierFlags { var mask: NSEvent.ModifierFlags = [] if spec.modifiers.contains(.command) { mask.insert(.command) } @@ -158,7 +226,10 @@ final class MenuBarController: NSObject { /// two. Drawn in code for the same reason `Tools/make-icon.swift` is, and the drawing handler /// is re-run per backing scale, so the strokes stay crisp on a Retina display rather than being /// a 16px bitmap doubled. - private static func icon() -> NSImage { + /// + /// `colour` is only for the flash, and it is what stops the image being a template: a template + /// is a mask, and a mask has no colour to give. + private static func icon(_ colour: NSColor? = nil) -> NSImage { let side: CGFloat = 16 let image = NSImage(size: NSSize(width: side, height: side), flipped: false) { _ in let r = side * 0.44 @@ -187,11 +258,11 @@ final class MenuBarController: NSObject { path.line(to: corner) } - NSColor.black.setStroke() + (colour ?? .black).setStroke() path.stroke() return true } - image.isTemplate = true + image.isTemplate = colour == nil return image } } diff --git a/Tests/FFMCoreTests/HotkeySpecTests.swift b/Tests/FFMCoreTests/HotkeySpecTests.swift index 2a0a259..ced0b66 100644 --- a/Tests/FFMCoreTests/HotkeySpecTests.swift +++ b/Tests/FFMCoreTests/HotkeySpecTests.swift @@ -78,4 +78,89 @@ final class HotkeySpecTests: XCTestCase { XCTAssertEqual(HotkeySpec("cmd+\(key)")?.keyCode, code, "key code for \(key)") } } + + // MARK: - Changing the modifier + + func testTheWrittenFormParsesBackToTheSameCombination() { + for text in ["cmd+ctrl+h", "⌘⌃⇧F5", "alt-command-left", "ctrl+opt+cmd+space"] { + let spec = HotkeySpec(text)! + XCTAssertEqual(HotkeySpec(spec.written), spec, "\(text) did not survive being written") + } + } + + func testTheKeyIsKeptWhenTheModifierChanges() { + let changed = HotkeySpec("cmd+ctrl+right")!.withModifiers([.shift, .command]) + XCTAssertEqual(changed?.display, "⇧⌘right".replacingOccurrences(of: "right", with: "Right")) + XCTAssertEqual(changed?.written, "shift+cmd+right") + } + + /// The same rule a typed combination has to pass: shift alone is not a chord, it is how a + /// capital letter is typed. + func testAModifierChangeThatWouldNotBeALegalHotkeyIsRefused() { + XCTAssertNil(HotkeySpec("cmd+ctrl+h")!.withModifiers([.shift])) + XCTAssertNil(HotkeySpec("cmd+ctrl+h")!.withModifiers([])) + } + + func testRewritingKeepsEachKey() { + XCTAssertEqual(rewriteHotkey("cmd+ctrl+h", modifiers: [.option, .command]), "alt+cmd+h") + XCTAssertEqual(rewriteHotkey("cmd+ctrl+left", modifiers: [.option, .command]), + "alt+cmd+left") + } + + /// A shortcut somebody switched off must not come back because they changed the modifier. + func testRewritingLeavesASwitchedOffShortcutAlone() { + XCTAssertEqual(rewriteHotkey("", modifiers: [.option, .command]), "") + XCTAssertEqual(rewriteHotkey(" ", modifiers: [.option, .command]), " ") + XCTAssertEqual(rewriteHotkey("none", modifiers: [.option, .command]), "none") + } + + /// Nor must a typo turn into a working shortcut nobody asked for. + func testRewritingLeavesSomethingItCannotParseAlone() { + XCTAssertEqual(rewriteHotkey("cmd+ctrl+nonsense", modifiers: [.option, .command]), + "cmd+ctrl+nonsense") + } + + /// The trap a "did this actually change anything?" check has to avoid. What is stored is + /// however somebody wrote it; what comes back from a rewrite is canonical. The same chord is + /// then two different strings, and comparing the text would call it a change. + func testTheSameCombinationCanBeWrittenTwoWays() { + let rewritten = rewriteHotkey("cmd+ctrl+h", modifiers: [.control, .command]) + XCTAssertNotEqual(rewritten, "cmd+ctrl+h", "the canonical order is not the typed one") + XCTAssertEqual(HotkeySpec(rewritten), HotkeySpec("cmd+ctrl+h"), + "but they are the same combination") + } + + // MARK: - What the menu offers + + func testEveryOfferedModifierMakesALegalHotkey() { + let spec = HotkeySpec("cmd+ctrl+right")! + for preset in ModifierPreset.allCases { + XCTAssertNotNil(spec.withModifiers(preset.modifiers), "\(preset.display) is not usable") + } + } + + func testTheOfferedModifiersAreAllDifferent() { + let sets = ModifierPreset.allCases.map(\.modifiers) + XCTAssertEqual(Set(sets.map { $0.map(\.rawValue).sorted().joined() }).count, sets.count) + } + + func testTheModifierInForceIsRecognised() { + XCTAssertEqual(ModifierPreset.matching([.control, .command]), .controlCommand) + XCTAssertEqual(ModifierPreset.matching(HotkeySpec("cmd+shift+left")!.modifiers), + .shiftCommand) + } + + /// Someone who typed their own combination into `defaults write` should see none of the offered + /// ones ticked, rather than the nearest. + func testAModifierNobodyOfferedMatchesNothing() { + XCTAssertNil(ModifierPreset.matching([.command])) + XCTAssertNil(ModifierPreset.matching([.control, .option, .shift, .command])) + XCTAssertNil(ModifierPreset.matching(nil)) + } + + func testTheCombinationsThatTakeSomethingAwaySaySo() { + XCTAssertNotNil(ModifierPreset.shiftCommand.caution) + XCTAssertNotNil(ModifierPreset.optionCommand.caution) + XCTAssertNil(ModifierPreset.controlCommand.caution) + } } From 684f5a9fc73325be2fdc171da6a96270933fb46b Mon Sep 17 00:00:00 2001 From: Richard Boisvert Date: Sat, 5 Sep 2026 17:08:13 -0400 Subject: [PATCH 2/2] chore(menu-bar): stop offering Command-Shift as a modifier Command-Shift with the arrow keys is how a line is selected in every text field on the system. A registered hotkey takes the combination away everywhere, and nothing refuses it -- the system reads those directly rather than registering them the way an app does -- so the menu would have been handing that away from behind a tooltip, which is not a warning anyone reads. `defaults write` still sets it for anyone who wants it. What is gone is only the one-click way to lose line selection by accident. --- README.md | 11 ++++++----- Sources/FFMCore/HotkeySpec.swift | 13 ++++++------- Tests/FFMCoreTests/HotkeySpecTests.swift | 14 +++++++++++--- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index acf6d36..c3bd0cb 100644 --- a/README.md +++ b/README.md @@ -83,11 +83,12 @@ it. Every shortcut moves together or none does, so they cannot end up under two Heed registers a hotkey exclusively, which takes the combination away from every other app. That is why the focus shortcuts default to Control and not Shift: Command+Shift+Left and Command+Shift+Right -are how a line is selected in every text field on the system, and Command+Option+Left and -Command+Option+Right move between tabs in most browsers and terminals. Both are offered in the menu -anyway, with a tooltip saying what they cost — but neither is the default. Heed can only refuse a -combination another app registered the same way; nothing refuses one the system reads directly, so -those two are taken quietly. +are how a line is selected in every text field on the system, so the menu does not offer that one at +all. Command+Option+Left and Command+Option+Right move between tabs in most browsers and terminals; +that one is offered, with a tooltip saying what it costs, but it is not the default. Either can still +be set with `defaults write`. Heed can only refuse a combination another app registered the same way +— nothing refuses one the system reads directly, so combinations like those are taken quietly, and a +warning before the fact is the only one possible. To hide the menu bar icon: diff --git a/Sources/FFMCore/HotkeySpec.swift b/Sources/FFMCore/HotkeySpec.swift index 63c8f39..9a8c4ec 100644 --- a/Sources/FFMCore/HotkeySpec.swift +++ b/Sources/FFMCore/HotkeySpec.swift @@ -134,13 +134,12 @@ public func rewriteHotkey(_ text: String, modifiers: Set) - /// change the modifier without going near `defaults write`, which remains there for anyone who /// wants something else. public enum ModifierPreset: CaseIterable, Sendable { - case controlCommand, optionCommand, shiftCommand, controlOption, controlOptionCommand + case controlCommand, optionCommand, controlOption, controlOptionCommand public var modifiers: Set { switch self { case .controlCommand: [.control, .command] case .optionCommand: [.option, .command] - case .shiftCommand: [.shift, .command] case .controlOption: [.control, .option] case .controlOptionCommand: [.control, .option, .command] } @@ -171,13 +170,13 @@ public enum ModifierPreset: CaseIterable, Sendable { /// /// Registering a hotkey takes the combination away from every other app, and Carbon only /// refuses one that *another app* claimed the same way. Nothing refuses a combination the - /// system reads directly, so these two are taken quietly and the only warning possible is this - /// one, before the fact. + /// system reads directly, so this is the only warning possible, before the fact. + /// + /// Command-Shift is not on the list above for the same reason and taken further: with the arrow + /// keys it is how a line is selected in every text field on the system, which is too much to + /// take away from behind a tooltip. `defaults write` will still set it for anyone who wants it. public var caution: String? { switch self { - case .shiftCommand: - "Command-Shift with the arrow keys selects to the start or end of a line in every text " - + "field. Heed would take that away." case .optionCommand: "Command-Option with the arrow keys moves between tabs in most browsers and terminals. " + "Heed would take that away." diff --git a/Tests/FFMCoreTests/HotkeySpecTests.swift b/Tests/FFMCoreTests/HotkeySpecTests.swift index ced0b66..d10b225 100644 --- a/Tests/FFMCoreTests/HotkeySpecTests.swift +++ b/Tests/FFMCoreTests/HotkeySpecTests.swift @@ -146,8 +146,8 @@ final class HotkeySpecTests: XCTestCase { func testTheModifierInForceIsRecognised() { XCTAssertEqual(ModifierPreset.matching([.control, .command]), .controlCommand) - XCTAssertEqual(ModifierPreset.matching(HotkeySpec("cmd+shift+left")!.modifiers), - .shiftCommand) + XCTAssertEqual(ModifierPreset.matching(HotkeySpec("cmd+ctrl+alt+left")!.modifiers), + .controlOptionCommand) } /// Someone who typed their own combination into `defaults write` should see none of the offered @@ -159,8 +159,16 @@ final class HotkeySpecTests: XCTestCase { } func testTheCombinationsThatTakeSomethingAwaySaySo() { - XCTAssertNotNil(ModifierPreset.shiftCommand.caution) XCTAssertNotNil(ModifierPreset.optionCommand.caution) XCTAssertNil(ModifierPreset.controlCommand.caution) } + + /// Command-Shift with the arrow keys selects a line in every text field on the system, which is + /// too much to take away from behind a tooltip. `defaults write` still sets it for anyone who + /// wants it; the menu does not hand it out. + func testCommandShiftIsNotOffered() { + XCTAssertFalse(ModifierPreset.allCases.contains { $0.modifiers == [.shift, .command] }) + XCTAssertNil(ModifierPreset.matching(HotkeySpec("cmd+shift+left")!.modifiers)) + XCTAssertNotNil(HotkeySpec("cmd+shift+left"), "but it is still a combination Heed accepts") + } }