feat(rules): give the global default a lock/switch action#57
Merged
Conversation
The global default was the only rule type stuck on one behavior — per-app, per-URL, and address-bar rules already choose lock vs one-shot switch. Add `LockConfiguration.defaultAction` (default `.lock`, fully backward compatible) so the global default can switch you into its source on entering any app with no rule of its own, then leave you free (re-firing per no-rule app), instead of only ever continuously pinning. Closes #56. - RuleResolver branches step 3 on defaultAction; the engine already routed .switchOnce(_, .globalDefault) through the frontmost switch slot, so it needed no change. - App Rules pane gains a Lock to / Switch to picker (shown once a source is set); reuses existing catalog keys, no new strings. - URL scheme: set-default-source takes an optional action=lock|switch (default lock); status/get-config surface defaultAction. - Backup/import round-trips defaultAction; the import diff now treats a same-source/different-action default as a reviewable conflict instead of silently dropping it. - Menu-bar source pick and the global cycle shortcuts set only the source and preserve the configured lock/switch mode — never silently flip it. - Docs (DESIGN, README, URL-Scheme-API) + all 8 translations updated. Tests: RuleResolver global-default switch, LockConfiguration decode/round- trip, LockEngine switch/lock/launcher scenarios, URLCommandParser action, ConfigBackup and ImportPlan (incl. same-source/different-action conflict).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughAdds configurable global default behavior with Sequence Diagram(s)sequenceDiagram
participant User
participant Settings
participant AppState
participant RuleResolver
participant InputSource
User->>Settings: Select global source and behavior
Settings->>AppState: Save source and defaultAction
AppState->>RuleResolver: Re-resolve foreground app
RuleResolver->>InputSource: Apply lock or switchOnce
InputSource-->>User: Updated input source
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
Tests/LockIMEKitTests/URLCommandParserTests.swift (1)
69-85: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover all accepted
actionaliases.The parser also accepts
switchonce,switch-once, and case-insensitive values, but this test only covers the canonicalswitchspelling. Add those expectations so the URL API compatibility contract cannot regress unnoticed. The parser contract is defined inSources/LockIMEKit/API/URLCommand.swift, Lines [260-340].Suggested additions
`#expect`(command("lockime://set-default-source?id=com.apple.keylayout.ABC&action=switch") == .setDefaultSource(source: .id(abc), action: .switchOnce)) + `#expect`(command("lockime://set-default-source?id=com.apple.keylayout.ABC&action=switchonce") + == .setDefaultSource(source: .id(abc), action: .switchOnce)) + `#expect`(command("lockime://set-default-source?id=com.apple.keylayout.ABC&action=switch-once") + == .setDefaultSource(source: .id(abc), action: .switchOnce)) + `#expect`(command("lockime://set-default-source?id=com.apple.keylayout.ABC&action=SWITCH") + == .setDefaultSource(source: .id(abc), action: .switchOnce))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LockIMEKitTests/URLCommandParserTests.swift` around lines 69 - 85, Add expectations to setDefaultSourceClear for the accepted switchOnce action aliases “switchonce” and “switch-once”, including mixed-case values, and assert each parses as action .switchOnce. Keep the existing canonical switch, lock, clear-path, and invalid-action coverage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/README/README.de.md`:
- Line 54: Überarbeite den deutschen Beschreibungstext zum Verhalten „Keine“ so,
dass klar ist: Es deaktiviert ausschließlich das globale Standardverhalten und
hat selbst keine globale Wirkung. Entferne die irreführende Aussage „als reiner
Umschalter pro App/pro Seite“, ohne die weiterhin möglichen Sperren durch App-
oder URL-Regeln zu verändern.
In `@docs/README/README.fr.md`:
- Line 54: Clarify the `Aucune` behavior in the documented “Verrouiller
globalement, ou simplement basculer” description: state that it disables only
global default behavior, while per-application and per-site rules may still lock
or switch input sources. Remove the misleading claim that it acts as a purely
per-application/per-site switch.
In `@docs/URL-Scheme-API/README.de.md`:
- Around line 109-112: Clarify the `set-default-source` effect description to
state that the global default is used by app rules that defer to it, only after
higher-priority URL and app rules are evaluated. Replace the narrower “when
entering an app without its own rule” wording while preserving the existing
distinction between `lock` and `switch` and the deletion-path behavior.
- Around line 109-112: Update the localized set-default-source descriptions to
state that the global default action applies when no higher-priority URL/app
binding or rule wins, including rules that defer to the global default. In
docs/URL-Scheme-API/README.de.md lines 109-112, replace the “App ohne eigene
Regel” condition; make the equivalent wording updates in
docs/README/README.ja.md line 54, README.pt.md line 58, README.ru.md line 54,
README.zh-CN.md line 57, and README.zh-TW.md line 54.
- Around line 213-217: Correct the quotation marks in the documentation sentence
describing the enabled switch, replacing the mismatched closing double quote
after “LockIME aktivieren” with the matching German closing quote while
preserving the surrounding text.
In `@README.md`:
- Around line 64-69: Clarify the global None state as disabling only global
default behavior, without implying that per-app or per-site rules are disabled.
Update the corresponding wording in README.md lines 64-69,
docs/README/README.de.md line 54, docs/README/README.es.md line 58, and
docs/README/README.fr.md line 54; replace each “pure per-app/per-site switcher”
translation with equivalent “no global behavior” wording.
In `@Sources/LockIMEKit/Backup/ConfigBackup.swift`:
- Line 241: Update the backup metadata associated with defaultAction in
ConfigBackup so switch-action backups require a reader version newer than 1, or
conditionally stamp the higher minReader only for non-lock actions. Preserve the
existing reader requirement for lock actions while ensuring older readers reject
backups whose semantics they cannot restore.
---
Nitpick comments:
In `@Tests/LockIMEKitTests/URLCommandParserTests.swift`:
- Around line 69-85: Add expectations to setDefaultSourceClear for the accepted
switchOnce action aliases “switchonce” and “switch-once”, including mixed-case
values, and assert each parses as action .switchOnce. Keep the existing
canonical switch, lock, clear-path, and invalid-action coverage unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b1bbee10-af88-4b80-a927-d97068bd445c
📒 Files selected for processing (35)
README.mdSources/LockIME/API/URLCommandHandler.swiftSources/LockIME/AppState.swiftSources/LockIME/UI/MenuBarView.swiftSources/LockIME/UI/Settings/AppRulesSettingsPane.swiftSources/LockIME/UI/Settings/ImportReviewSheet.swiftSources/LockIMEKit/API/URLCommand.swiftSources/LockIMEKit/Backup/ConfigBackup.swiftSources/LockIMEKit/Backup/ImportPlan.swiftSources/LockIMEKit/Rules/LockConfiguration.swiftSources/LockIMEKit/Rules/RuleResolver.swiftTests/LockIMEKitTests/ConfigBackupTests.swiftTests/LockIMEKitTests/ImportPlanTests.swiftTests/LockIMEKitTests/LockConfigurationTests.swiftTests/LockIMEKitTests/LockEngineTests.swiftTests/LockIMEKitTests/RuleResolverTests.swiftTests/LockIMEKitTests/URLCommandParserTests.swiftdocs/DESIGN.mddocs/README/README.de.mddocs/README/README.es.mddocs/README/README.fr.mddocs/README/README.ja.mddocs/README/README.pt.mddocs/README/README.ru.mddocs/README/README.zh-CN.mddocs/README/README.zh-TW.mddocs/URL-Scheme-API/README.de.mddocs/URL-Scheme-API/README.es.mddocs/URL-Scheme-API/README.fr.mddocs/URL-Scheme-API/README.ja.mddocs/URL-Scheme-API/README.mddocs/URL-Scheme-API/README.pt.mddocs/URL-Scheme-API/README.ru.mddocs/URL-Scheme-API/README.zh-CN.mddocs/URL-Scheme-API/README.zh-TW.md
Address the CodeRabbit review on the global-default docs: - Describe a global default of None as having no global behavior (per-app and per-URL rules still apply, and can still lock a source) instead of "pinning nothing" / "a pure per-app/per-site switcher". - Describe switch as firing whenever an app falls back to the global default (no rule of its own, or a rule set to Use default; after higher-priority URL/app rules), not only apps with no rule of their own. - Fix a mismatched German quotation mark in the URL-Scheme-API status section. Applies to README and the URL-Scheme-API reference across all translations.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/README/README.fr.md`:
- Line 54: In the user-facing description of the three global behaviors, update
the French phrase in the “Verrouiller globalement, ou simplement basculer”
bullet from “l'un de trois comportements” to the idiomatic “l'un des trois
comportements,” preserving the rest of the text unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e3f449e7-50e3-4a9e-9868-d03bf5cc9f67
📒 Files selected for processing (18)
README.mddocs/README/README.de.mddocs/README/README.es.mddocs/README/README.fr.mddocs/README/README.ja.mddocs/README/README.pt.mddocs/README/README.ru.mddocs/README/README.zh-CN.mddocs/README/README.zh-TW.mddocs/URL-Scheme-API/README.de.mddocs/URL-Scheme-API/README.es.mddocs/URL-Scheme-API/README.fr.mddocs/URL-Scheme-API/README.ja.mddocs/URL-Scheme-API/README.mddocs/URL-Scheme-API/README.pt.mddocs/URL-Scheme-API/README.ru.mddocs/URL-Scheme-API/README.zh-CN.mddocs/URL-Scheme-API/README.zh-TW.md
🚧 Files skipped from review as they are similar to previous changes (13)
- docs/README/README.es.md
- docs/URL-Scheme-API/README.md
- docs/URL-Scheme-API/README.es.md
- docs/README/README.de.md
- docs/README/README.zh-CN.md
- docs/README/README.ru.md
- docs/URL-Scheme-API/README.zh-TW.md
- docs/URL-Scheme-API/README.zh-CN.md
- docs/URL-Scheme-API/README.de.md
- docs/README/README.ja.md
- README.md
- docs/URL-Scheme-API/README.ru.md
- docs/URL-Scheme-API/README.ja.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The global default source was the only rule type locked to a single behavior — per-app, per-URL, and address-bar rules already let you choose between a continuous lock and a one-shot switch, but the global default could only pin. This adds
LockConfiguration.defaultAction(default.lock, fully backward compatible) so it gains the same choice: in switch mode LockIME switches you into the default source when you enter any app with no rule of its own, then steps out of the way and lets you change it — re-firing on each new no-rule app. That is distinct from a global default of None, which does nothing globally; switch actively switches you in on every no-rule app.The engine needed no change —
LockEnginealready routed a.switchOnce(_, .globalDefault)resolution through its frontmost switch slot (keyed by bundle ID, so it re-fires per app) — so the core is justRuleResolverbranching on the new action. The rest threads it through the surfaces that already understand lock-vs-switch:set-default-sourcetakes an optionalaction=lock|switch(defaultlock);status/get-configreportdefaultAction.One deliberate interaction choice: the menu-bar source pick and the global cycle shortcuts set only the source and preserve the configured lock/switch mode — they never silently flip a switch default to lock. The mode stays a Settings decision.
Docs (DESIGN, README, URL-Scheme-API) and all eight translations are updated in step. New tests cover the resolver branch, lenient
defaultActiondecoding, the LockEngine switch/lock/launcher scenarios from the issue, the URL parser, and the backup/import round-trip including the same-source/different-action conflict.Closes #56