feat: Hyprland Wayland support + fix wl-copy clipboard timeout#416
feat: Hyprland Wayland support + fix wl-copy clipboard timeout#416FelipeAfonso wants to merge 6 commits intoOpenWhispr:mainfrom
Conversation
these were added by claude opus after writing to the md file
Changes are Hyprland-only except for the shared native shortcut logic.
|
@gabrielste1n Tested and pushed a couple of commits on top (hotkey validation, Hyprland detection logging). Feel free to give it a look and merge when ready. |
gabrielste1n
left a comment
There was a problem hiding this comment.
Code Quality Review
Solid contribution — the Hyprland integration mirrors the GNOME shortcut pattern well, and isUsingNativeShortcut() is a good generalization. A few issues worth addressing, roughly ordered by impact:
1. wl-copy timeout scoped to Hyprland only — likely wrong
const isHyprland = !!process.env.HYPRLAND_INSTANCE_SIGNATURE;
const result = spawnSync("wl-copy", ["--", text], { timeout: isHyprland ? 50 : 1 });The PR description says: "wl-copy forks to become the Wayland clipboard owner. The previous 1ms timeout killed it before the fork completed." That's wl-copy behavior, not Hyprland-specific. Other wlroots compositors (Sway, river, wayfire) using wl-copy will have the same problem. clipboard.js already has isWlrootsCompositor() (line 25) — use that, or just bump the timeout universally.
2. API contract inconsistency between GNOME and Hyprland managers
GNOME manager takes pre-converted format (caller converts in hotkeyManager.js):
const gnomeHotkey = GnomeShortcutManager.convertToGnomeFormat(hotkey);
const success = await this.gnomeManager.registerKeybinding(gnomeHotkey);Hyprland manager takes Electron format and converts internally:
const success = await this.hyprlandManager.registerKeybinding(hotkey);This means validation also happens at different layers — GNOME validates the output format, Hyprland validates the input format. Future maintainers will assume these two managers share the same API contract.
3. Capture mode duplication in ipcHandlers.js
set-hotkey-listening-mode now has two near-identical blocks for GNOME and Hyprland on both entry and exit:
if (hotkeyManager.isUsingGnome() && hotkeyManager.gnomeManager) {
await hotkeyManager.gnomeManager.unregisterKeybinding().catch(...);
}
if (hotkeyManager.isUsingHyprland() && hotkeyManager.hyprlandManager) {
await hotkeyManager.hyprlandManager.unregisterKeybinding().catch(...);
}Since isUsingNativeShortcut() already exists, a single code path through the hotkey manager (e.g., hotkeyManager.unregisterNativeShortcut()) would eliminate this and prevent it from growing with each new compositor.
Nits
Double validation in registerKeybinding: Both isValidHotkey() (regex) and convertToHyprlandFormat() (returns null) reject invalid hotkeys. The regex is a redundant gate — GNOME only validates once.
updateKeybinding is meaningless indirection: GNOME's updateKeybinding has real logic (checks isRegistered, only updates the binding field). Hyprland's just calls registerKeybinding. Fine for API symmetry, but worth a comment.
Unused fields in convertToHyprlandFormat return: Returns { mods, key, bindKey } but only bindKey is ever consumed. mods and key are dead API surface.
Unrelated diff noise: debugLogger.error reformatting in ipcHandlers.js (~L997, ~L1819) and blank line removal in CLAUDE.md model registry section.
D-Bus name sharing: Both managers claim com.openwhispr.App. Control flow prevents collision (GNOME returns early), but a comment noting the mutual-exclusivity assumption would help.
What's good
- Faithful mirror of GNOME's D-Bus toggle pattern
- Proper detection chain (env var + XDG fallback)
- Ephemeral bindings — no persistent Hyprland config mutation
timeout: 5000onhyprctlcalls is actually better than GNOME's no-timeoutgsettings- Clean cleanup in
close()anddestroy() - Graceful fallback to
globalShortcuton failure
gabrielste1n
left a comment
There was a problem hiding this comment.
Code Quality Review
Solid contribution — the Hyprland integration mirrors the GNOME shortcut pattern well, and isUsingNativeShortcut() is a good generalization. A few issues worth addressing, roughly ordered by impact:
1. wl-copy timeout scoped to Hyprland only — likely wrong
const isHyprland = !!process.env.HYPRLAND_INSTANCE_SIGNATURE;
const result = spawnSync("wl-copy", ["--", text], { timeout: isHyprland ? 50 : 1 });The PR description says: "wl-copy forks to become the Wayland clipboard owner. The previous 1ms timeout killed it before the fork completed." That's wl-copy behavior, not Hyprland-specific. Other wlroots compositors (Sway, river, wayfire) using wl-copy will have the same problem. clipboard.js already has isWlrootsCompositor() (line 25) — use that, or just bump the timeout universally.
2. API contract inconsistency between GNOME and Hyprland managers
GNOME manager takes pre-converted format (caller converts in hotkeyManager.js):
const gnomeHotkey = GnomeShortcutManager.convertToGnomeFormat(hotkey);
const success = await this.gnomeManager.registerKeybinding(gnomeHotkey);Hyprland manager takes Electron format and converts internally:
const success = await this.hyprlandManager.registerKeybinding(hotkey);This means validation also happens at different layers — GNOME validates the output format, Hyprland validates the input format. Future maintainers will assume these two managers share the same API contract.
3. Capture mode duplication in ipcHandlers.js
set-hotkey-listening-mode now has two near-identical blocks for GNOME and Hyprland on both entry and exit:
if (hotkeyManager.isUsingGnome() && hotkeyManager.gnomeManager) {
await hotkeyManager.gnomeManager.unregisterKeybinding().catch(...);
}
if (hotkeyManager.isUsingHyprland() && hotkeyManager.hyprlandManager) {
await hotkeyManager.hyprlandManager.unregisterKeybinding().catch(...);
}Since isUsingNativeShortcut() already exists, a single code path through the hotkey manager (e.g., hotkeyManager.unregisterNativeShortcut()) would eliminate this and prevent it from growing with each new compositor.
Nits
Double validation in registerKeybinding: Both isValidHotkey() (regex) and convertToHyprlandFormat() (returns null) reject invalid hotkeys. The regex is a redundant gate — GNOME only validates once.
updateKeybinding is meaningless indirection: GNOME's updateKeybinding has real logic (checks isRegistered, only updates the binding field). Hyprland's just calls registerKeybinding. Fine for API symmetry, but worth a comment.
Unused fields in convertToHyprlandFormat return: Returns { mods, key, bindKey } but only bindKey is ever consumed. mods and key are dead API surface.
Unrelated diff noise: debugLogger.error reformatting in ipcHandlers.js (~L997, ~L1819) and blank line removal in CLAUDE.md model registry section.
D-Bus name sharing: Both managers claim com.openwhispr.App. Control flow prevents collision (GNOME returns early), but a comment noting the mutual-exclusivity assumption would help.
What's good
- Faithful mirror of GNOME's D-Bus toggle pattern
- Proper detection chain (env var + XDG fallback)
- Ephemeral bindings — no persistent Hyprland config mutation
timeout: 5000onhyprctlcalls is actually better than GNOME's no-timeoutgsettings- Clean cleanup in
close()anddestroy() - Graceful fallback to
globalShortcuton failure
Summary
hyprctlkeybindings + D-Bus, matching the existing GNOME Wayland approachwl-copyclipboard write failing silently on Wayland due to a 1msspawnSynctimeout — increased to 50ms so the fork completes before being killedDetails
Hyprland shortcuts: New
hyprlandShortcut.jshelper detects Hyprland viaHYPRLAND_INSTANCE_SIGNATURE, registers runtime keybindings withhyprctl keyword bind, and receives toggle events over D-Bus. UI components now use a genericisUsingNativeShortcutflag to hide the activation mode selector on both GNOME and Hyprland.Clipboard fix:
wl-copyforks to become the Wayland clipboard owner. The previous 1ms timeout killed it before the fork completed, so the clipboard was never set on Wayland. Paste simulation would then paste stale clipboard content instead of the transcription. Tested reliably at 50ms.