Global Hotkey Docs #1548
Global Hotkey Docs #1548
Conversation
Renames the globalhotkey setting
|
Warning Rate limit exceeded@oneirocosm has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 47 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe pull request introduces a new system-wide global hotkey configuration for Wave, shifting from a "key:" prefix to an "app:" prefix in configuration settings. This change spans multiple files across documentation, frontend types, configuration constants, and utility functions. The modification allows users to set a custom keybinding to focus on the most recent Wave window from anywhere on their computer, with enhanced support for various key mappings and improved documentation. Changes
Sequence DiagramsequenceDiagram
participant User
participant App
participant Settings
participant ElectronKeyMapper
User->>Settings: Set global hotkey
Settings->>App: Configure app:globalhotkey
App->>ElectronKeyMapper: Convert key mapping
ElectronKeyMapper-->>App: Translated key code
App->>User: Enable global hotkey functionality
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
emain/emain-util.ts
Fixed
| @@ -170,6 +170,9 @@ | |||
| export function waveKeyToElectronKey(waveKey: string): string { | |||
| const waveParts = waveKey.split(":"); | |||
| const electronParts: Array<string> = waveParts.map((part: string) => { | |||
| const digitRegexpMatch = new RegExp("^c\{Digit(\d)\}$").exec(part); | |||
Check failure
Code scanning / CodeQL
Useless regular-expression character escape High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 17 days ago
To fix the problem, we need to remove the unnecessary escape sequence \{ from the regular expression on line 173. This will make the regular expression clearer and easier to understand without changing its functionality.
- In general terms, we should ensure that only necessary escape sequences are used in regular expressions.
- Specifically, we will update the regular expression on line 173 to remove the unnecessary escape sequence
\{. - No additional methods, imports, or definitions are needed to implement this change.
| @@ -172,3 +172,3 @@ | ||
| const electronParts: Array<string> = waveParts.map((part: string) => { | ||
| const digitRegexpMatch = new RegExp("^c\{Digit(\d)\}$").exec(part); | ||
| const digitRegexpMatch = new RegExp("^c{Digit(\d)}$").exec(part); | ||
| const numpadRegexpMatch = new RegExp("^c\{Numpad(\d)\}$").exec(part); |
emain/emain-util.ts
Fixed
| @@ -170,6 +170,9 @@ | |||
| export function waveKeyToElectronKey(waveKey: string): string { | |||
| const waveParts = waveKey.split(":"); | |||
| const electronParts: Array<string> = waveParts.map((part: string) => { | |||
| const digitRegexpMatch = new RegExp("^c\{Digit(\d)\}$").exec(part); | |||
Check failure
Code scanning / CodeQL
Useless regular-expression character escape High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 17 days ago
To fix the problem, we need to ensure that the backslash is correctly interpreted as part of the regular expression. This can be achieved by using double backslashes (\\d) within the string literal to ensure that the regular expression correctly interprets it as a digit character class.
- Update the regular expression string literals to use double backslashes for the digit character class.
- Specifically, change the lines where the regular expressions are defined to use
\\dinstead of\d.
| @@ -172,4 +172,4 @@ | ||
| const electronParts: Array<string> = waveParts.map((part: string) => { | ||
| const digitRegexpMatch = new RegExp("^c\{Digit(\d)\}$").exec(part); | ||
| const numpadRegexpMatch = new RegExp("^c\{Numpad(\d)\}$").exec(part); | ||
| const digitRegexpMatch = new RegExp("^c\\{Digit(\\d)\\}$").exec(part); | ||
| const numpadRegexpMatch = new RegExp("^c\\{Numpad(\\d)\\}$").exec(part); | ||
| const lowercaseCharMatch = new RegExp("^([a-z])$").exec(part); |
emain/emain-util.ts
Fixed
| @@ -170,6 +170,9 @@ | |||
| export function waveKeyToElectronKey(waveKey: string): string { | |||
| const waveParts = waveKey.split(":"); | |||
| const electronParts: Array<string> = waveParts.map((part: string) => { | |||
| const digitRegexpMatch = new RegExp("^c\{Digit(\d)\}$").exec(part); | |||
Check failure
Code scanning / CodeQL
Useless regular-expression character escape High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 17 days ago
To fix the problem, we need to remove the unnecessary escape sequence \} from the regular expression on line 173. This will make the code more readable and maintainable without changing its functionality.
| @@ -172,3 +172,3 @@ | ||
| const electronParts: Array<string> = waveParts.map((part: string) => { | ||
| const digitRegexpMatch = new RegExp("^c\{Digit(\d)\}$").exec(part); | ||
| const digitRegexpMatch = new RegExp("^c{Digit(\d)}$").exec(part); | ||
| const numpadRegexpMatch = new RegExp("^c\{Numpad(\d)\}$").exec(part); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
pkg/wconfig/metaconsts.go (1)
9-10: LGTM! Consider documenting the prefix change.The new constants follow the established naming convention and are properly grouped. The transition from "key:" to "app:" prefix is a logical change that better represents the configuration's purpose.
Consider adding a comment explaining the rationale behind the "app:" prefix transition for future maintainers.
docs/docs/keybindings.mdx (1)
74-77: Fix typo and consider enhancing the documentation.
- "Customizeable" should be "Customizable"
- Consider adding a basic example of setting a global hotkey to make the documentation more immediately useful.
-## Customizeable Systemwide Global Hotkey +## Customizable Systemwide Global Hotkeyemain/emain-util.ts (1)
188-241: Consider refactoring key mappings for better maintainability.The current implementation uses a long series of if statements. Consider using a mapping object for better maintainability and performance.
const KEY_MAPPINGS = { 'Soft1': 'F21', 'Soft2': 'F22', 'Soft3': 'F23', 'Soft4': 'F24', ' ': 'Space', 'CapsLock': 'Capslock', 'NumLock': 'Numlock', 'ScrollLock': 'Scrolllock', 'AudioVolumeUp': 'VolumeUp', 'AudioVolumeDown': 'VolumeDown', 'AudioVolumeMute': 'VolumeMute', 'MediaTrackNext': 'MediaNextTrack', 'MediaTrackPrevious': 'MediaPreviousTrack', 'Decimal': 'numdec', 'Add': 'numadd', 'Subtract': 'numsub', 'Multiply': 'nummult', 'Divide': 'numdiv', }; // Then in the code: if (KEY_MAPPINGS[part]) { return KEY_MAPPINGS[part]; }docs/docs/config.mdx (3)
208-208: Add missing comma for better readability.-Soft keys `Soft1` through `Soft4`. These are essentially the same as `F21` through `F24`. +Soft keys `Soft1` through `Soft4`. These are essentially the same as `F21` through `F24`,🧰 Tools
🪛 LanguageTool
[uncategorized] ~208-~208: Possible missing comma found.
Context: ...ally the same asF21throughF24. - Space represented as a literal space <c...(AI_HYDRA_LEO_MISSING_COMMA)
197-231: Consider reorganizing the key list for better readability.The key list is comprehensive but could be better organized into logical groups. Consider structuring it as follows:
-// Current flat list +// Organized list +#### Modifier Keys +- `Ctrl`, `Cmd`, `Shift`, `Alt`, `Option`, `Meta`, `Super` + +#### Standard Keys +- Letters: `a` through `z` +- Digits: `c{Digit0}` through `c{Digit9}` +- Function Keys: `F1` through `F20` +- Soft Keys: `Soft1` through `Soft4` (same as `F21` through `F24`) + +#### Special Keys +- Navigation: `ArrowUp`, `ArrowDown`, `ArrowLeft`, `ArrowRight`, `Home`, `End`, `PageUp`, `PageDown` +- Control: `Enter`, `Tab`, `Backspace`, `Delete`, `Insert`, `Esc`, `Space` +- Lock Keys: `CapsLock`, `NumLock` + +#### Media Keys +- Volume: `AudioVolumeUp`, `AudioVolumeDown`, `AudioVolumeMute` +- Playback: `MediaTrackNext`, `MediaTrackPrevious`, `MediaPlayPause`, `MediaStop` + +#### Numpad Keys +- Numbers: `c{Numpad0}` through `c{Numpad9}` +- Operators: `Add`, `Subtract`, `Multiply`, `Divide`, `Decimal`🧰 Tools
🪛 LanguageTool
[uncategorized] ~208-~208: Possible missing comma found.
Context: ...ally the same asF21throughF24. - Space represented as a literal space <c...(AI_HYDRA_LEO_MISSING_COMMA)
232-232: Consider adding more examples for better clarity.The current example is good but could be expanded to show more common use cases.
Add more examples:
Common examples: - Windows/Linux: `"Ctrl:Alt:w"` for Ctrl+Alt+W - macOS: `"Cmd:Shift:w"` for Command+Shift+W - Function key: `"F12"` - Multiple modifiers: `"Ctrl:Shift:Alt:t"`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/docs/config.mdx(2 hunks)docs/docs/keybindings.mdx(1 hunks)emain/emain-util.ts(2 hunks)emain/emain.ts(1 hunks)frontend/types/gotypes.d.ts(1 hunks)pkg/wconfig/metaconsts.go(1 hunks)pkg/wconfig/settingsconfig.go(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/docs/config.mdx
[uncategorized] ~208-~208: Possible missing comma found.
Context: ...ally the same as F21 through F24. - Space represented as a literal space <c...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Biome (1.9.4)
emain/emain-util.ts
[error] 173-173: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
(lint/complexity/useRegexLiterals)
[error] 174-174: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
(lint/complexity/useRegexLiterals)
[error] 175-175: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
(lint/complexity/useRegexLiterals)
🔇 Additional comments (4)
pkg/wconfig/settingsconfig.go (1)
36-37: LGTM! Configuration structure changes look good.
The new fields AppClear and AppGlobalHotkey are properly defined with correct types and JSON tags, maintaining consistency with the configuration structure.
Let's verify that all references to the old configuration keys have been updated:
✅ Verification successful
Let me verify the remaining part about key:clear vs app:clear to ensure complete consistency.
Let me try a different approach to verify the clear configuration by searching for boolean fields in the configuration types.
Let me try one final verification to check for any Clear-related configuration in metaconsts.go since we found the other configuration key there.
Configuration changes are properly integrated across the codebase
The verification shows that:
- The new
app:globalhotkeyis consistently used in the frontend (emain.ts), types (gotypes.d.ts), configuration (settingsconfig.go), constants (metaconsts.go), and is properly documented (config.mdx) - The
AppClearfield withapp:*JSON tag follows the same pattern as other clear configurations in the codebase (likeAiClear,TermClear, etc.) - No references to the old
key:prefixed configurations remain
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to the old key:globalhotkey
rg "key:globalhotkey"
# Search for references to the new app:globalhotkey to ensure proper adoption
rg "app:globalhotkey"
Length of output: 1005
Script:
#!/bin/bash
# Search for any remaining references to the old key:clear
rg "key:clear"
# Search for references to the new app:clear to ensure proper adoption
rg "app:clear"
Length of output: 29
Script:
#!/bin/bash
# Search for AppClear usage in the codebase
ast-grep --pattern 'AppClear'
# Search for any Clear-related configuration in TypeScript types
rg "clear" frontend/types/gotypes.d.ts
# Search for Clear-related configuration in documentation
rg -i "clear" docs/docs/config.mdx
Length of output: 141
Script:
#!/bin/bash
# Check metaconsts.go for Clear-related configuration
rg "Clear" pkg/wconfig/metaconsts.go
# Also check for any other potential references to Clear in the Go files
rg "Clear" pkg/wconfig/
Length of output: 2287
emain/emain.ts (1)
614-614: LGTM! Configuration key update is correct.
The change from key:globalhotkey to app:globalhotkey aligns with the new configuration structure.
docs/docs/config.mdx (1)
28-28: LGTM! Configuration documentation is clear and well-placed.
The new configuration key is properly documented in the configuration table with a clear description and reference to the detailed section.
frontend/types/gotypes.d.ts (1)
609-610: LGTM! Consistent naming pattern for the new app namespace.
The new properties follow the established pattern for settings configuration and appropriately replace the old key:* and key:globalhotkey properties.
Let's verify that all usages of the old properties have been migrated:
✅ Verification successful
Migration to app:globalhotkey is complete and consistent
The new app:globalhotkey property is properly integrated across the codebase:
- Defined in TypeScript types (
frontend/types/gotypes.d.ts) - Implemented in Go settings config (
pkg/wconfig/settingsconfig.go) - Constants defined in
pkg/wconfig/metaconsts.go - Used in electron main process (
emain/emain.ts) - Documented in config docs (
docs/docs/config.mdx)
No references to the old key:globalhotkey property were found, confirming a complete migration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining usage of the old key:globalhotkey property
rg "key:globalhotkey" -l
# Search for any remaining usage of the old key:* property
rg "key:\\*" -l
# Search for usage of the new app:globalhotkey property to ensure it's being used
rg "app:globalhotkey" -l
Length of output: 186
Script:
#!/bin/bash
# Let's check the actual content of these files to understand the usage
echo "=== pkg/wconfig/settingsconfig.go ==="
rg -A 5 "app:globalhotkey" pkg/wconfig/settingsconfig.go
echo -e "\n=== pkg/wconfig/metaconsts.go ==="
rg -A 5 "app:globalhotkey" pkg/wconfig/metaconsts.go
echo -e "\n=== emain/emain.ts ==="
rg -A 5 "app:globalhotkey" emain/emain.ts
echo -e "\n=== docs/docs/config.mdx ==="
rg -A 5 "app:globalhotkey" docs/docs/config.mdx
# Also check if there are any remaining references to the old key:globalhotkey
echo -e "\n=== Checking for old key:globalhotkey references ==="
rg -A 5 "key:globalhotkey"
Length of output: 3531
Requires matching on the range [0-9] instead of \d. Also grabs the match instead of the entire expression.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
emain/emain-util.ts (2)
173-175: Use RegExp literals for better readability and maintainability.Replace RegExp constructors with literal notation to improve code readability and reduce escaping complexity.
-const digitRegexpMatch = new RegExp("^c{Digit([0-9])}$").exec(part); -const numpadRegexpMatch = new RegExp("^c{Numpad([0-9])}$").exec(part); -const lowercaseCharMatch = new RegExp("^([a-z])$").exec(part); +const digitRegexpMatch = /^c{Digit([0-9])}$/.exec(part); +const numpadRegexpMatch = /^c{Numpad([0-9])}$/.exec(part); +const lowercaseCharMatch = /^([a-z])$/.exec(part);🧰 Tools
🪛 Biome (1.9.4)
[error] 173-173: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.(lint/complexity/useRegexLiterals)
[error] 174-174: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.(lint/complexity/useRegexLiterals)
[error] 175-175: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.(lint/complexity/useRegexLiterals)
242-244: Consider removing or guarding debug logging.The console.log statement for digit matches might not be necessary in production code. Consider removing it or guarding it behind a debug flag.
if (digitRegexpMatch && digitRegexpMatch.length > 1) { - console.log("digit match is ", digitRegexpMatch); return digitRegexpMatch[1]; }docs/docs/config.mdx (1)
195-234: Minor documentation improvements needed.The documentation is comprehensive, but there are a few minor improvements to consider:
- Change "anywhere in your computer" to "anywhere on your computer"
- Add a comma after "F24" in the Soft keys description
- Consider adding more examples for common use cases
-Wave allows settings a custom global hotkey to open your most recent window from anywhere in your computer. +Wave allows settings a custom global hotkey to open your most recent window from anywhere on your computer. -Soft keys `Soft1` through `Soft4`. These are essentially the same as `F21` through `F24`. +Soft keys `Soft1` through `Soft4`. These are essentially the same as `F21` through `F24`,🧰 Tools
🪛 LanguageTool
[uncategorized] ~197-~197: The preposition “on” seems more likely in this position than the preposition “in”.
Context: ...n your most recent window from anywhere in your computer. This has the name `"app:...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_IN_ON)
[uncategorized] ~210-~210: Possible missing comma found.
Context: ...ally the same asF21throughF24. - Space represented as a literal space <c...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/docs/config.mdx(2 hunks)emain/emain-util.ts(2 hunks)frontend/types/gotypes.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/types/gotypes.d.ts
🧰 Additional context used
🪛 Biome (1.9.4)
emain/emain-util.ts
[error] 173-173: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
(lint/complexity/useRegexLiterals)
[error] 174-174: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
(lint/complexity/useRegexLiterals)
[error] 175-175: Use a regular expression literal instead of the RegExp constructor.
Regular expression literals avoid some escaping required in a string literal, and are easier to analyze statically.
Safe fix: Use a literal notation instead.
(lint/complexity/useRegexLiterals)
🪛 LanguageTool
docs/docs/config.mdx
[uncategorized] ~197-~197: The preposition “on” seems more likely in this position than the preposition “in”.
Context: ...n your most recent window from anywhere in your computer. This has the name `"app:...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_IN_ON)
[uncategorized] ~210-~210: Possible missing comma found.
Context: ...ally the same as F21 through F24. - Space represented as a literal space <c...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (3)
emain/emain-util.ts (2)
188-199: LGTM: Clear and consistent Soft keys mapping.
The mapping of Soft1-Soft4 to F21-F24 is implemented correctly and aligns with the documentation.
200-226: LGTM: Comprehensive special keys mapping.
The implementation correctly maps various special keys to their corresponding Electron accelerator key names, providing good coverage for media and system keys.
docs/docs/config.mdx (1)
28-28: LGTM: Well-documented configuration entry.
The app:globalhotkey configuration entry is clearly documented with appropriate cross-referencing to the detailed section.
Documents the allowed keys for globalhotkey. Also improves the algorithm for computing electron keybinds from wave keybindings.
Summary by CodeRabbit
New Features
Documentation
Chores