feat: separate the app master switch from locking#39
Merged
Conversation
The single "Enable input-source locking" toggle gated everything —
both the continuous lock and the one-shot switch rules. Turning it
off to avoid a global lock silently disabled per-app and per-URL
switch rules too, so LockIME could not be used as a pure switcher
(the Input Source Pro model the issue asks for).
Split the model in two: the master `isEnabled` ("Enable LockIME")
gates the whole app, and a new subordinate `lockingEnabled`
("Enable locking", on by default) gates only continuous locking.
One-shot switching is now gated by the master alone, so master on
plus locking off pins nothing while per-app/per-site switch rules
keep firing. `LockConfiguration` decodes an absent `lockingEnabled`
as `true`, so existing configs and the lock-on default are
unchanged.
The menu bar, the toggle-lock shortcut, the `lockime://` API (new
`set-locking` command; `status` now reports `enabled` and
`lockingEnabled`), and the README / URL-Scheme-API docs in all nine
languages are updated to match.
Closes #37
Signed-off-by: Kevin Cui <bh@bugs.cc>
|
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 (9)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (2)
Summary by CodeRabbit
WalkthroughThe PR separates LockIME master enablement from continuous locking. Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPane
participant AppState
participant LockEngine
participant URLCommandHandler
User->>SettingsPane: toggle Enable LockIME / Enable locking
SettingsPane->>AppState: setMasterEnabled / setLockingEnabled
AppState->>LockEngine: apply(reason:)
LockEngine->>LockEngine: reevaluate / enable or disable enforcement
User->>URLCommandHandler: lockime://set-locking?enabled=...
URLCommandHandler->>AppState: update lockingEnabled
AppState->>LockEngine: apply(reason:)
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches✨ Simplify code
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.22.1)docs/URL-Scheme-API/README.de.mdmarkdownlint-cli2 wrapper config was not available before execution docs/URL-Scheme-API/README.es.mdmarkdownlint-cli2 wrapper config was not available before execution docs/URL-Scheme-API/README.fr.mdmarkdownlint-cli2 wrapper config was not available before execution
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: 3
🤖 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/URL-Scheme-API/README.md`:
- Around line 81-94: Clarify the runtime behavior of the master/sub-toggle in
the Enable & locking section: the current wording in the `set-locking`
description implies no effect while the master is off, but `lockingEnabled` is
still persisted and reported by `status`. Update the copy around `lock`,
`unlock`, `toggle-lock`, and `set-locking` to explicitly distinguish immediate
runtime effect from persisted observable state, so readers understand that
`set-locking` can still change state even when `LockIME` is off.
In `@docs/URL-Scheme-API/README.pt.md`:
- Around line 97-98: In the Portuguese README translation, update the wording
for the alias labels in the table so it uses “aliás” instead of “alias”; adjust
the entries for the `toggle-lock` and `set-locking` rows in `README.pt.md` to
keep the translation natural and consistent with the rest of the document.
In `@Sources/LockIME/UI/MenuBarView.swift`:
- Around line 55-61: The checked-row clear path in MenuBarView should key off
the configured default target, not state.isLocked, so the current saved source
remains selectable even when locking is disabled. Update the isLockedTo
condition in MenuBarView’s Button action to use state.config.defaultSourceID ==
source.id as the source of truth, and keep the clear action on
state.setDefaultSource(nil) tied to that target. This ensures the “click checked
source” path still clears the stored defaultSourceID regardless of
lockingEnabled.
🪄 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: c89bae3c-8b13-447e-90de-6e4a6ff09794
📒 Files selected for processing (31)
README.mdSources/LockIME/API/URLCommandHandler.swiftSources/LockIME/AppState.swiftSources/LockIME/Localizable.xcstringsSources/LockIME/UI/MenuBarView.swiftSources/LockIME/UI/Settings/GeneralSettingsPane.swiftSources/LockIMEKit/API/URLCommand.swiftSources/LockIMEKit/Backup/ImportPlan.swiftSources/LockIMEKit/LockEngine/LockEngine.swiftSources/LockIMEKit/Rules/LockConfiguration.swiftTests/LockIMEKitTests/LockConfigurationTests.swiftTests/LockIMEKitTests/LockEngineTests.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
set-locking persists lockingEnabled (and status reports it), so it still changes observable state while the master is off — it simply has no immediate runtime effect until the master is on. Sync the wording across all nine URL-Scheme-API docs. Signed-off-by: Kevin Cui <bh@bugs.cc>
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 single "Enable input-source locking" toggle gated everything — both the continuous lock and the one-shot switch rules — so turning it off to avoid a global lock silently disabled per-app and per-URL switch rules too. That left no way to run LockIME as a pure switcher (the Input Source Pro model the issue asks for): you either locked everything or got nothing.
This splits the model into two levels. A master
isEnabled("Enable LockIME") gates the whole app, and a new subordinatelockingEnabled("Enable locking", on by default) gates only the continuous lock — the global default, per-app.lockedrules, URL.lockrules, and the address-bar lock. One-shot switching is now gated by the master alone, so master on + locking off pins nothing while per-app/per-site switch rules keep firing. In the General pane the second toggle sits under the master and dims when it's off (the HIG subordinate-control pattern).Backward compatible:
LockConfigurationdecodes an absentlockingEnabledastrue, so existing configs — and the lock-on default — behave exactly as before. The only users whose behavior changes are the ones the issue is about: those who had the master off with switch rules configured, whose switching now starts working.Also updated to match: the menu bar (clicking the checked source clears just the global lock target instead of disabling the app), the toggle-lock shortcut (flips the master), the
lockime://API (newset-lockingcommand;statusnow reportsenabledandlockingEnabledalongsidelocked), and the README + URL-Scheme-API docs across all nine languages.The decoupling truth table is covered by new engine tests, plus migration-default and
set-lockingparser tests.Closes #37