Skip to content

Allow user to configure push-to-talk shortcut from panel#16

Open
vikvang wants to merge 1 commit intofarzaa:mainfrom
vikvang:feature/configurable-ptt-shortcut
Open

Allow user to configure push-to-talk shortcut from panel#16
vikvang wants to merge 1 commit intofarzaa:mainfrom
vikvang:feature/configurable-ptt-shortcut

Conversation

@vikvang
Copy link
Copy Markdown

@vikvang vikvang commented Apr 8, 2026

Summary

The push-to-talk keyboard shortcut was hardcoded to Ctrl+Option. Users may prefer a different modifier combination to avoid conflicts with other apps or match their muscle memory. This PR makes it user-configurable via a dropdown picker in the companion panel.

Changes

BuddyDictationManager.swift

  • Made ShortcutOption a String-backed, CaseIterable enum so it can be persisted to UserDefaults and iterated in the picker
  • Added pickerLabel property for human-readable display in the dropdown
  • Changed currentShortcutOption from a static let to a computed property that reads the user's preference from UserDefaults (defaulting to Ctrl+Option)
  • Made pushToTalkDisplayText and pushToTalkTooltipText computed so they reflect the current shortcut

CompanionManager.swift

  • Added selectedPushToTalkModifierCombination published property backed by UserDefaults
  • Added setSelectedPushToTalkModifierCombination() setter that persists the choice and resets the monitor's pressed state

CompanionPanelView.swift

  • Added a shortcut picker dropdown (styled to match the existing model picker) that appears when onboarding is complete and all permissions are granted
  • Updated instruction text ("Hold X to talk") to dynamically reflect the selected shortcut

GlobalPushToTalkShortcutMonitor.swift

  • Added resetShortcutPressedState() to prevent the old shortcut from getting stuck in a pressed state when the user switches combinations

How it works

The shortcut takes effect immediately — no app restart needed. Each CGEvent tap callback reads currentShortcutOption (which reads from UserDefaults), so changing the preference is picked up on the next keypress automatically.

Co-Authored-By: Oz oz-agent@warp.dev

The push-to-talk keyboard shortcut was hardcoded to Ctrl+Option. Users may
prefer a different modifier combination to avoid conflicts with other apps
or match their muscle memory.

- Make ShortcutOption String-backed and CaseIterable so choices can be
  persisted to UserDefaults and enumerated in the picker UI
- Change currentShortcutOption from a static let to a computed property
  that reads the user's preference (defaulting to Ctrl+Option)
- Add a dropdown shortcut picker to the companion panel, styled to match
  the existing model picker
- Update the instruction text to reflect the currently selected shortcut
- Reset the monitor's pressed state on shortcut change to prevent the old
  combo from getting stuck in a pressed state

Co-Authored-By: Oz <oz-agent@warp.dev>
@Ambisphaeric
Copy link
Copy Markdown

Pulled this in, switching to Shift+Fn worked well for me.

@qodo-ai-reviewer
Copy link
Copy Markdown

Hi, BuddyPushToTalkShortcut.shortcutTransition now reads UserDefaults via currentShortcutOption on every global key event, potentially multiple times per event. This adds avoidable work on the CGEvent tap hot path and can increase input latency while the monitor is running.

Severity: remediation recommended | Category: performance

How to fix: Cache option; update on change

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

BuddyPushToTalkShortcut.currentShortcutOption hits UserDefaults on every keyboard event processed by the global event tap (and may be evaluated multiple times per event).

Issue Context

This is in the hot path (GlobalPushToTalkShortcutMonitor.handleGlobalEventTap), so avoid per-event UserDefaults reads.

Fix Focus Areas

  • leanring-buddy/BuddyDictationManager.swift[113-214]
  • leanring-buddy/GlobalPushToTalkShortcutMonitor.swift[120-137]

Suggested approach

  • Introduce a cached in-memory shortcut option (e.g., static var cachedShortcutOption defaulting to .controlOption).
  • Update the cache when the user changes the shortcut (e.g., in CompanionManager.setSelectedPushToTalkModifierCombination, set the cache, or post a notification that updates it).
  • Ensure shortcutTransition uses the cached option without calling into UserDefaults.

We noticed a couple of other issues in this PR as well - happy to share if helpful.


Found by Qodo. Free code review for open-source maintainers.

@qodo-ai-reviewer
Copy link
Copy Markdown

Hi, selectedPushToTalkModifierCombination is publicly settable, so it can be mutated without persisting to UserDefaults or resetting the monitor pressed state. This can desynchronize the UI selection from the actual shortcut behavior and can reintroduce stuck-pressed state if changed while pressed.

Severity: remediation recommended | Category: maintainability

How to fix: Make selection setter-only API

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

selectedPushToTalkModifierCombination is a mutable @Published var with required side effects (persisting to UserDefaults and resetting pressed state) implemented in a separate method.

Issue Context

Accidental direct assignment to the @Published property would skip persistence and the pressed-state reset.

Fix Focus Areas

  • leanring-buddy/CompanionManager.swift[110-120]

Suggested approach

  • Make the property @Published private(set) and expose changes only via setSelectedPushToTalkModifierCombination.
  • Alternatively, replace it with a computed property backed by UserDefaults (similar to selectedModel pattern), but ensure you still reset the monitor pressed state on changes.
  • Optionally rename the method to setPushToTalkShortcutOption to clarify it’s the only supported mutation path.

Spotted by Qodo code review - free for open-source projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants