Make settings configurable from the command line - #289
Conversation
Shortcuts and voice macros are stored as JSON-encoded Data. 'defaults write' cannot produce a Data value without hex-encoding the payload first, so these settings were effectively unreachable from a shell script, a dotfiles repo, or a Nix module. Worse, a plain string was discarded silently: data(forKey:) returned nil, the binding fell back to the default, and the default was written back over the user's value. Read through a helper that falls back to string(forKey:). Writes still use Data, so the stored format is unchanged and this is backwards compatible in both directions.
📝 WalkthroughWalkthroughChangesConfiguration Support
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
Sources/AppState.swift (1)
927-943: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression tests for both storage representations.
Test valid JSON stored as
Dataand as a UTF-8Stringfor shortcuts and voice macros. Add malformed-input coverage to preserve the intended fallback behavior.This follows the new Data/string configuration contract.
🤖 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 `@Sources/AppState.swift` around lines 927 - 943, Extend the regression tests for shortcut and voice-macro loading to cover valid JSON stored both as UserDefaults Data and as a UTF-8 String, using the existing test helpers and configuration keys. Add malformed-input cases for each representation and assert the established fallback behavior, while preserving the existing valid Data-path expectations.
🤖 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 `@README.md`:
- Around line 204-208: Update the README shortcut lookup instructions to reflect
that hold_shortcut is stored as Data and defaults read returns hexadecimal
property-list bytes, not copyable JSON. Add a decode step or clearly explain how
to convert the output before using it with -string, referencing the persistence
behavior in AppState.swift.
- Around line 116-120: Update the README settings documentation to cover both
credential locations: the default FreeFlow path and the bundle-name-derived path
used by builds such as “FreeFlow Dev.” At each relevant setup or access step,
document both paths or use a shell variable derived from the current
CFBundleName instead of assuming only `FreeFlow/.settings`.
- Around line 214-223: Update the README credential setup commands so the
settings file is protected before JSON is written: set a restrictive umask and
create or truncate ~/Library/Application\ Support/FreeFlow/.settings with mode
600 before the cat write. Preserve the existing file path, credentials, and
final configuration while eliminating the permissive creation window.
- Around line 198-200: Update the README voice macro JSON example to include the
required id field alongside command and payload, ensuring it can be decoded by
JSONDecoder into VoiceMacro without being discarded.
---
Nitpick comments:
In `@Sources/AppState.swift`:
- Around line 927-943: Extend the regression tests for shortcut and voice-macro
loading to cover valid JSON stored both as UserDefaults Data and as a UTF-8
String, using the existing test helpers and configuration keys. Add
malformed-input cases for each representation and assert the established
fallback behavior, while preserving the existing valid Data-path expectations.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 44a3202e-635f-47b0-a2b8-e34f35466a4d
📒 Files selected for processing (2)
README.mdSources/AppState.swift
Swift's synthesized decoder ignores the default value on 'id' and fails with keyNotFound when the key is absent, so a hand-written macro list decoded to nothing at all - silently, since the failure just yields an empty array. Decode 'id' with decodeIfPresent and fall back to a fresh UUID. Encoding still writes 'id', so stored macros round-trip unchanged.
Reading a shortcut back with 'defaults read' prints the property-list data representation as hex rather than the JSON to copy, so document the decode. Create the credentials file with a restrictive umask before writing to it, rather than chmod-ing afterwards when the key is already on disk.
|
Thanks, good round. Three of the four were valid and are fixed; one I've left with a reason in-thread. Voice macro
Credentials file permissions (5c6f3fc) - correct, reproduced the window at Both The PR description is updated, including a correction to the verification section that previously overstated what the macro test had shown. |
1 participant
Makes every FreeFlow setting reachable from a shell, so a machine can be configured from dotfiles, a setup script, or a Nix or Homebrew module instead of by clicking through Settings.
Most settings already worked this way, since they live in
UserDefaults:Three things stopped that from being a complete story.
1. Shortcuts and voice macros were unreachable, and failed silently
hold_shortcut,toggle_shortcut,copy_again_shortcut, the threesaved_*_custom_shortcutkeys andvoice_macrosare stored as JSON-encodedData.defaultscannot write aDatavalue without hex-encoding the payload first:Forcing it with
-stringstores the value, butloadShortcutreads it withUserDefaults.standard.data(forKey:), which returnsnilfor a string. The result is worse than an error: the binding falls back to the default and the default is then written back over the user's value, so a scripted setting silently disappears.loadShortcutand thevoice_macrosread now go through a helper that falls back tostring(forKey:). Writes still useData, so the stored format is unchanged and existing installs are unaffected. Hex-encoded-datavalues keep working too.2. A hand-written voice macro could not decode at all
VoiceMacro.idhas a default value, but Swift's synthesized decoder ignores defaults and fails withkeyNotFoundwhen the key is absent. Since the failure just yields an empty array, a hand-written macro list silently produced no macros:idis now decoded withdecodeIfPresentand falls back to a freshUUID(), so onlycommandandpayloadare required. Explicit ids are still honoured andencodestill writesid, so stored macros round-trip unchanged.3. Nothing documented any of this
Added a README section covering all 45 user-configurable keys with their types, the shortcut and macro JSON, and the
.settingsfile that holds API credentials. It also states the constraint that matters most in practice: FreeFlow reads settings once at launch and does not watch for changes, so you have to quit before writing and start afterwards.Verification
Tested against a real build using the
FreeFlow Devbundle, so the separatecom.zachlatta.freeflow.devdomain kept my own settings out of it. The dev domain was backed up and restored afterwards.A/B, writing
hold_shortcutas a plain JSON string for Right Command while the built-in default is Fn:keyCode: 63(Fn) asData- string discarded, default written over itkeyCode: 54(Right Command) - string honouredThe
VoiceMacrodecoding change is covered by a harness built from the struct as it appears inAppState.swift, checking that a macro withoutiddecodes and gets a generated UUID, that an explicitidis preserved, and thatencodestill emitsid.The credentials file permissions were checked by reproducing the old sequence (
644with the key already on disk beforechmod) against the new one (600throughout).The
FnJSON in the README is the app's own output, read back after letting FreeFlow write its default, rather than something I hand-wrote.Not included
I deliberately did not add a config-file layer or live reloading. macOS already provides the declarative layer through
defaultsand managed preferences, and live reload would need care around thedidSetwrite-back racing external edits. Happy to look at either if you'd want them.