feat: add support for custom aliases #150
Conversation
…sh data retrieval
…reflect recent changes
…itial layout state
…al layouts based on advanced settings
📝 WalkthroughWalkthroughAdds support for custom key aliases in OverKeys through documentation, configuration models, and key event detection. Extends UserConfig, KeyboardState, and ConfigService to handle custom aliases, implements loading during configuration initialization, and adds logic to detect alias key combinations during key press handling. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant MethodCallHandler
participant ConfigurationLoader
participant ConfigService
participant KeyboardNotifier
participant KeyEventService
App->>MethodCallHandler: handleMethodCall(loadAllConfiguration)
MethodCallHandler->>ConfigurationLoader: loadAllConfiguration()
ConfigurationLoader->>ConfigService: getCustomAliases()
ConfigService-->>ConfigurationLoader: Map<String, List<String>>
ConfigurationLoader->>KeyboardNotifier: updateCustomAliases(aliases)
KeyboardNotifier-->>ConfigurationLoader: state updated
Note over KeyEventService: On key press
KeyEventService->>KeyboardNotifier: read keyboard state
KeyEventService->>KeyEventService: iterate customAliases
rect rgb(230, 245, 230)
Note over KeyEventService: For each alias, check if all<br/>constituent keys are pressed
KeyEventService->>KeyEventService: compute alias trigger condition
end
KeyEventService->>KeyboardNotifier: updateCustomAliases state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
lib/services/method_call_handler.dart (1)
45-45: Good refactoring - consolidates configuration loading.Adding the
loadAllConfigurationparameter is a solid improvement that consolidates multiple configuration loading calls into a single, coordinated operation. This reduces duplication and ensures all configuration is loaded consistently.lib/services/config_service.dart (1)
13-13: Static cache may cause unexpected shared state across instances.Changing
_cachedConfigto a static field means allConfigServiceinstances share the same cache. WhileclearCache()still works, instantiating multipleConfigServiceobjects now shares state unexpectedly. If singleton behavior is intended, consider makingConfigServicea singleton explicitly for clarity.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/advanced/custom-aliases.mddocs/examples/sample_config.jsondocs/index.mdlib/app.dartlib/models/user_config.dartlib/providers/keyboard_provider.dartlib/services/config_service.dartlib/services/configuration_loader.dartlib/services/key_event_service.dartlib/services/method_call_handler.dart
🧰 Additional context used
🪛 Biome (2.1.2)
docs/examples/sample_config.json
[error] 95-95: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 95-96: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 96-96: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🪛 LanguageTool
docs/advanced/custom-aliases.md
[style] ~70-~70: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... key labeled "Item 1" will light up.
- When you press
X, the key labeled "ULT" wi...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (14)
docs/examples/sample_config.json (1)
92-98: LGTM! Static analysis warnings are false positives.The custom aliases configuration looks good and provides useful examples for different use cases (system commands like UNDO/PASTE and gaming shortcuts). The static analysis errors from Biome are false positives—this file uses JSONC format (JSON with Comments), as noted on line 4, which supports trailing commas and comments that strict JSON parsers reject.
lib/app.dart (1)
510-510: LGTM! Properly connects configuration loading.The addition of
_loadConfigurationas a parameter correctly integrates the new consolidated configuration loading flow into the method call handler. The implementation (lines 166-170) appropriately clears the cache before loading, ensuring fresh configuration data.lib/services/method_call_handler.dart (1)
429-433: LGTM! Cleaner configuration loading flow.The refactored approach using a single
loadAllConfiguration()call is much cleaner than the previous implementation that required multiple conditional config loads. This delegates the responsibility for loading order and conditional logic to the ConfigurationLoader, which is the appropriate place for such logic.docs/index.md (1)
24-24: Documentation file exists with appropriate content.The
docs/advanced/custom-aliases.mdfile is present and contains clear setup instructions with a practical JSON configuration example for defining custom key aliases. The link in the documentation index is valid and properly integrated.lib/services/configuration_loader.dart (1)
50-53: Unconditional loading of custom aliases is intentional and properly implemented.Both
getCustomAliases()andupdateCustomAliases()methods exist and are correctly implemented. The unconditional loading of custom aliases matches the design pattern ofloadCustomShiftMappings—both are treated as basic configuration features loaded before theadvancedSettingsEnabledcheck. Advanced features (user layouts, alt layouts, custom fonts, Kanata) are the only ones gated byadvancedSettingsEnabledon line 26. Custom aliases are actively used in key event handling inkey_event_service.dart, confirming this is the intended behavior.docs/advanced/custom-aliases.md (1)
1-75: Documentation looks comprehensive and well-structured.The setup instructions, configuration details, and usage examples clearly explain the custom aliases feature. The documentation correctly describes the modifier handling (Control, Shift, Alt, Win) which matches the implementation in
key_event_service.dart.lib/services/key_event_service.dart (2)
324-338: LGTM!The held layer release logic correctly handles the edge case where a user toggles another layer while holding a held-layer key. The comment clearly explains the scenario.
109-112: No changes needed. TheWinmodifier handling is correct.The Windows key mapping in
key_code.dartdefinesVK_LWIN→'Win'(not'LWin') andVK_RWIN→'RWin'. The code at lines 109-112 correctly checks for'Win'and'RWin', matching the actual key mappings returned by the key code utilities.Likely an incorrect or invalid review comment.
lib/services/config_service.dart (1)
146-162: LGTM!The
getCustomAliases()method properly loads and converts the aliases with appropriate error handling. The try-catch ensures malformed configuration data doesn't crash the application.lib/providers/keyboard_provider.dart (3)
59-59: LGTM!The
customAliasesfield is properly integrated intoKeyboardStatewith consistent handling in the constructor andcopyWithmethod, following the established patterns for other similar fields.Also applies to: 111-111, 164-164, 220-220
350-357: LGTM!The
fromJsondeserialization correctly handles the nested structure, converting the dynamic list values toList<String>. The pattern is consistent with other map fields in the class.
373-375: LGTM!The
updateCustomAliasesmethod follows the same pattern as other updaters inKeyboardNotifier.lib/models/user_config.dart (2)
12-12: LGTM!The
customAliasesfield is properly added toUserConfigwith appropriate JSON parsing. UsingMap<String, dynamic>?at the config layer is appropriate since the type conversion toMap<String, List<String>>?is handled byConfigService.getCustomAliases().Also applies to: 23-23, 53-56, 67-67
95-96: LGTM!The
toJsoncorrectly conditionally includescustomAliasesonly when non-null and non-empty, consistent with other optional fields.
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.