feat!: collapse the two-level enable model into one master switch#50
Merged
Conversation
The subordinate "Enable locking" toggle (added by #39 for issue #37) made the app carry two states for one concept: a global default of None already expresses "switch on entry, pin nothing", so the sub-toggle was redundant. One `Enable LockIME` switch now gates everything, and the tray header reads Enabled/Disabled instead of Locked/Unlocked. `LockConfiguration.lockingEnabled` is gone. A legacy config that explicitly persisted `lockingEnabled: false` (a v1.5.x pure-switch user) migrates by clearing `defaultSourceID`, so the upgrade cannot silently re-pin their input source everywhere. Rule-level locks (`.locked` app rules, `.lock` URL/address-bar rules) deliberately re-engage instead of being demoted: rules now mean what they say, and rewriting the user's rule modes on a heuristic would be the more destructive migration. The decode shim is marked `TODO(legacy-locking-migration)` for removal a few releases from now. The README and the URL Scheme API reference are updated across all nine languages, and the catalog swaps the Locked/Unlocked keys for Enabled/Disabled with translations for every supported language. BREAKING CHANGE: the `set-locking` URL command (alias `locking`) now fails with `unknown_command`, and the `status` payload no longer carries `lockingEnabled`/`locked` — read `enabled` instead. 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 (2)
Summary by CodeRabbit
WalkthroughThis PR removes the 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 1
🧹 Nitpick comments (3)
docs/README/README.es.md (1)
136-136: 📐 Maintainability & Code Quality | 🔵 TrivialPoint this cross-link at the canonical English reference.
This translated README should link to the English URL Scheme API doc to stay aligned with the docs guideline and avoid per-locale drift.
♻️ Proposed fix
- Referencia completa: **[URL Scheme API](../URL-Scheme-API/README.es.md)**. + Referencia completa: **[URL Scheme API](../URL-Scheme-API/README.md)**.As per coding guidelines, "Intra-doc links in translated READMEs should point at English docs using relative paths (
../DESIGN.md,../RELEASING.md,../../README.md,../../LICENSE)."🤖 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 `@docs/README/README.es.md` at line 136, The cross-link in the translated README still points to the Spanish URL Scheme API doc instead of the canonical English reference. Update the Markdown link in README.es.md for the URL Scheme API entry to use the English target, following the same relative-path pattern used by other translated docs, and keep the link text unchanged.Source: Coding guidelines
docs/README/README.de.md (1)
132-132: 📐 Maintainability & Code Quality | 🔵 TrivialPoint this cross-link at the canonical English reference.
This translated README should link to the English URL Scheme API doc to stay aligned with the docs guideline and avoid per-locale drift.
♻️ Proposed fix
- **[URL Scheme API](../URL-Scheme-API/README.de.md)** + **[URL Scheme API](../URL-Scheme-API/README.md)**As per coding guidelines, "Intra-doc links in translated READMEs should point at English docs using relative paths (
../DESIGN.md,../RELEASING.md,../../README.md,../../LICENSE)."🤖 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 `@docs/README/README.de.md` at line 132, The cross-link in the translated README points to the localized URL Scheme API doc instead of the canonical English reference. Update the Markdown link in README.de.md so it targets the English URL-Scheme-API README using the appropriate relative path, consistent with the docs guideline for translated READMEs and the existing link style in this section.Source: Coding guidelines
docs/README/README.fr.md (1)
132-132: 📐 Maintainability & Code Quality | 🔵 TrivialPoint this cross-link at the canonical English reference.
This translated README should link to the English URL Scheme API doc to stay aligned with the docs guideline and avoid per-locale drift.
♻️ Proposed fix
- Référence complète : **[URL Scheme API](../URL-Scheme-API/README.fr.md)**. + Référence complète : **[URL Scheme API](../URL-Scheme-API/README.md)**.As per coding guidelines, "Intra-doc links in translated READMEs should point at English docs using relative paths (
../DESIGN.md,../RELEASING.md,../../README.md,../../LICENSE)."🤖 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 `@docs/README/README.fr.md` at line 132, The cross-link in README.fr.md points to the French URL Scheme API page instead of the canonical English doc. Update the reference so the translated README links to the English URL-Scheme-API README using the relative path convention used elsewhere in translated docs, keeping the link target aligned with the docs guideline.Source: Coding guidelines
🤖 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 `@Sources/LockIMEKit/LockEngine/LockEngine.swift`:
- Around line 134-151: apply(false) is still leaving URL polling and address-bar
monitoring active because LockEngine.apply(_:reason:) always calls
updateURLPolling() and updateAddressBarMonitoring(). Update those helpers, or
gate their calls from apply(_:reason:), so they no-op or tear down observers
when config.isEnabled is false and only start monitoring when the master switch
is on.
---
Nitpick comments:
In `@docs/README/README.de.md`:
- Line 132: The cross-link in the translated README points to the localized URL
Scheme API doc instead of the canonical English reference. Update the Markdown
link in README.de.md so it targets the English URL-Scheme-API README using the
appropriate relative path, consistent with the docs guideline for translated
READMEs and the existing link style in this section.
In `@docs/README/README.es.md`:
- Line 136: The cross-link in the translated README still points to the Spanish
URL Scheme API doc instead of the canonical English reference. Update the
Markdown link in README.es.md for the URL Scheme API entry to use the English
target, following the same relative-path pattern used by other translated docs,
and keep the link text unchanged.
In `@docs/README/README.fr.md`:
- Line 132: The cross-link in README.fr.md points to the French URL Scheme API
page instead of the canonical English doc. Update the reference so the
translated README links to the English URL-Scheme-API README using the relative
path convention used elsewhere in translated docs, keeping the link target
aligned with the docs guideline.
🪄 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: 4bed137c-1197-43d7-b890-a101cd7cd981
📒 Files selected for processing (34)
README.mdSources/LockIME/API/URLCommandHandler.swiftSources/LockIME/AppState.swiftSources/LockIME/Localizable.xcstringsSources/LockIME/LockIMEApp.swiftSources/LockIME/Shortcuts/ShortcutNames.swiftSources/LockIME/UI/AboutView.swiftSources/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
`apply()` re-ran `updateURLPolling()`/`updateAddressBarMonitoring()` on every config change, but neither helper guarded on `config.isEnabled`, so a disabled LockIME kept polling browser URLs and kept its AX address-bar observer attached — observing when it could never act. Pre-existing behavior, but the single-switch model makes "off ⇒ fully idle" the documented contract, so gate both helpers on the master. 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 "Enable locking" sub-toggle (#39, for issue #37) gave the app two states for one concept: a global default of None already expresses "switch on entry, pin nothing", so keeping a separate locking switch on top of Enable LockIME was redundant. This collapses the model back to the single switch — it gates everything, the General pane loses the sub-toggle, and the tray/menu header now reads Enabled/Disabled instead of Locked/Unlocked (new catalog keys, translated for every supported language).
LockConfiguration.lockingEnabledis removed. A config that explicitly persistedlockingEnabled: false(a v1.5.x pure-switch user) migrates on first decode by clearingdefaultSourceID, so the upgrade cannot silently re-pin their input source everywhere. Rule-level locks (.lockedapp rules,.lockURL/address-bar rules) deliberately re-engage instead of being demoted — rules now mean what they say, and rewriting the user's rule modes on a heuristic would be the more destructive migration. The decode shim is markedTODO(legacy-locking-migration)and can be deleted a few releases from now, once nobody is upgrading straight from v1.5.x.The README and the URL Scheme API reference are updated across all nine languages.
Breaking API change: the
set-lockingURL command (aliaslocking) now fails withunknown_command, and thestatuspayload no longer carrieslockingEnabled/locked— readenabledinstead.Refs #37