Skip to content

[macOS] Fix global shortcuts: F-key keycodes and headless event processing #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lijy91 merged 2 commits into from
Mar 1, 2026
+88 −8
Merged

[macOS] Fix global shortcuts: F-key keycodes and headless event processing #39

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
111f83f
Initial plan
Copilot Mar 1, 2026
7d7d185
Fix macOS global shortcuts: F-key lookup table and background event t…
Copilot Mar 1, 2026
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
96 changes: 88 additions & 8 deletions src/platform/macos/shortcut_manager_macos.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
#include <algorithm>
#include <atomic>
#include <cctype>
#include <mutex>
#include <string>
#include <thread>
#include <unordered_map>
#include <vector>
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ParseAcceleratorMac uses std::size(kFKeyCodes) but this file doesn't include a header that guarantees std::size is declared (C++17 places it in <iterator>). This can fail to compile on some toolchains; include the appropriate header (e.g. <iterator>/<array>) or avoid std::size here (e.g. std::extent_v / sizeof-based length).

Suggested change
#include <vector>
#include <vector>
#include <iterator>

Copilot uses AI. Check for mistakes.

Expand Down Expand Up @@ -138,12 +141,26 @@ bool ParseAcceleratorMac(const std::string& accelerator, UInt32& modifiers, UInt
}
}

if (key_token.rfind("f", 0) == 0) {
int fnum = std::stoi(key_token.substr(1));
if (fnum >= 1 && fnum <= 24) {
keycode = kVK_F1 + (fnum - 1);
return true;
if (key_token.rfind("f", 0) == 0 && key_token.size() > 1) {
// Carbon virtual key codes for function keys are non-contiguous; use explicit table.
static const UInt32 kFKeyCodes[] = {
kVK_F1, kVK_F2, kVK_F3, kVK_F4, kVK_F5,
kVK_F6, kVK_F7, kVK_F8, kVK_F9, kVK_F10,
kVK_F11, kVK_F12, kVK_F13, kVK_F14, kVK_F15,
kVK_F16, kVK_F17, kVK_F18, kVK_F19, kVK_F20,
};
try {
int fnum = std::stoi(key_token.substr(1));
if (fnum >= 1 && fnum <= static_cast<int>(std::size(kFKeyCodes))) {
keycode = kFKeyCodes[fnum - 1];
return true;
}
} catch (const std::invalid_argument&) {
return false;
} catch (const std::out_of_range&) {
return false;
}
return false;
}

if (key_token == "space") {
Expand Down Expand Up @@ -189,11 +206,17 @@ bool ParseAcceleratorMac(const std::string& accelerator, UInt32& modifiers, UInt

} // namespace

// Polling interval for the background event thread (seconds).
static constexpr EventTimeout kEventPollTimeout = kEventDurationSecond * 0.1;

class ShortcutManagerImpl final : public ShortcutManager::Impl {
public:
explicit ShortcutManagerImpl(ShortcutManager* manager) : manager_(manager) {}

~ShortcutManagerImpl() override {
StopThread();
JoinThread();

for (const auto& [id, hotkey] : hotkeys_) {
UnregisterEventHotKey(hotkey);
}
Expand All @@ -208,7 +231,7 @@ explicit ShortcutManagerImpl(ShortcutManager* manager) : manager_(manager) {}
bool IsSupported() override { return true; }

bool RegisterShortcut(const std::shared_ptr<Shortcut>& shortcut) override {
EnsureHandler();
EnsureThread();

UInt32 modifiers = 0;
UInt32 keycode = 0;
Expand Down Expand Up @@ -242,10 +265,10 @@ bool UnregisterShortcut(const std::shared_ptr<Shortcut>& shortcut) override {
return true;
}

void SetupEventMonitoring() override { EnsureHandler(); }
void SetupEventMonitoring() override { EnsureThread(); }

void CleanupEventMonitoring() override {
// Keep handler installed as long as shortcuts exist.
// Keep thread running while shortcuts may still be registered.
}

private:
Expand Down Expand Up @@ -279,6 +302,59 @@ void EnsureHandler() {
&handler_);
}

void EnsureThread() {
std::lock_guard<std::mutex> lock(thread_mutex_);
if (running_.load()) {
return;
}

EnsureHandler();

running_.store(true);
thread_ = std::thread([this]() { ThreadMain(); });
}

void StopThread() {
std::lock_guard<std::mutex> lock(thread_mutex_);
if (!running_.load()) {
return;
}

running_.store(false);
// Unlock before joining so the thread can complete any in-progress work.
// Note: the thread only touches running_ and Carbon APIs, so this is safe.
Comment on lines +318 to +325
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StopThread() only flips running_ and returns, leaving thread_ joinable. If EnsureThread() is ever called after StopThread() but before JoinThread(), thread_ = std::thread(...) would overwrite a joinable std::thread and call std::terminate. Consider making shutdown atomic by moving/swapping thread_ into a local under thread_mutex_, then joining that local outside the lock; also the comment about joining is currently misleading since StopThread() doesn’t join.

Suggested change
std::lock_guard<std::mutex> lock(thread_mutex_);
if (!running_.load()) {
return;
}
running_.store(false);
// Unlock before joining so the thread can complete any in-progress work.
// Note: the thread only touches running_ and Carbon APIs, so this is safe.
std::thread local_thread;
{
std::lock_guard<std::mutex> lock(thread_mutex_);
if (!running_.load()) {
return;
}
running_.store(false);
// Move the thread object out while holding the lock so that thread_
// is no longer joinable when EnsureThread() next runs.
local_thread = std::move(thread_);
}
// Join outside the lock so the worker can finish any in-progress work.
if (local_thread.joinable()) {
local_thread.join();
}

Copilot uses AI. Check for mistakes.
}

// Must be called without thread_mutex_ held (to allow the thread to finish).
void JoinThread() {
if (thread_.joinable()) {
thread_.join();
}
}

void ThreadMain() {
const EventTypeSpec hotkey_spec = {kEventClassKeyboard, kEventHotKeyPressed};

while (running_.load()) {
EventRef event = nullptr;
// Use a short timeout so the loop can check the running_ flag periodically.
OSStatus status =
ReceiveNextEvent(1, &hotkey_spec, kEventPollTimeout, true, &event);

if (!running_.load()) {
if (status == noErr && event) {
ReleaseEvent(event);
}
break;
}

if (status == noErr && event) {
SendEventToEventTarget(event, GetApplicationEventTarget());
ReleaseEvent(event);
}
}
}

void HandleHotKey(ShortcutId shortcut_id) {
auto shortcut = manager_->Get(shortcut_id);
if (!shortcut) {
Expand All @@ -296,6 +372,10 @@ void HandleHotKey(ShortcutId shortcut_id) {
ShortcutManager* manager_;
std::unordered_map<ShortcutId, EventHotKeyRef> hotkeys_;
EventHandlerRef handler_ = nullptr;

std::atomic<bool> running_{false};
std::thread thread_;
std::mutex thread_mutex_;
};

ShortcutManager::ShortcutManager()
Expand Down