fix(config): accept settingsPath keys in config --set - #277
vishnujayvel wants to merge 1 commit into
Conversation
findField only matched FieldDef.key (env-var style), so the settingsPath names used by the docs' config --set examples and by --plain --json (e.g. searchBackend) returned unknown_key. Resolve by key first, then settingsPath.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe headless ChangesHeadless setting resolution
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 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 |
|
Thanks for the careful review — particularly for confirming the Nothing further from me; happy to adjust if you'd rather the exclusion be explicit in code instead |
|
Thanks @Frankie-Xu for the detailed reviews and the independent verification runs on |
|
Hi @vishnujayvel — thank you for this PR, and I'm sorry it's been sitting here without a proper review. I've been heads-down building some pretty big features and improvements for wigolo, and as a solo dev, shipping that while also reviewing and maintaining every issue and PR has been genuinely hard. This one isn't forgotten — it just hasn't had the attention it deserves yet. I'd really appreciate your patience here. I'm going to work through the open PRs and issues properly over the next few weeks (sooner if I can free up), and I'll follow up right here. In the meantime, the wigolo Discord is open if you'd like to follow what's being built, ask questions, or nudge me directly: https://discord.gg/BkUUgz2bNF Thanks again for contributing, and for understanding — it genuinely means a lot. 🙏 |
What & why
Fixes #256. Reported by @tizerluo.
wigolo config --set searchBackend=hybridreturnedUnknown settingeven though that exact command is the documented example:Root cause:
applyHeadlessSet→findField()only matchedFieldDef.key(env-var style, e.g.WIGOLO_SEARCH— the names text--plainprints). The docs'--setexamples and the--plain --jsonoutput useFieldDef.settingsPath(e.g.searchBackend), which was never checked, so the documented--setform was rejected asunknown_key.Changes
findFieldinsrc/cli/tui/actions/headless-set.ts: exact match onFieldDef.keyfirst, then exact match onFieldDef.settingsPath.searchBackend=hybrid→status: 'ok'and save; env-styleWIGOLO_SEARCHstill works.No changes to secret handling, coercion, validation, or propagation.
Out of scope
The issue also mentions
tlsTier/WIGOLO_TLS_TIER. That env var has noconfig.json/settingsPathkey today, so accepting it via--setwould be new config surface — left as a maintainer decision, not part of this PR.Testing
applyHeadlessSet({ key: 'searchBackend', value: 'hybrid', ... })returnedstatus: 'unknown_key'(1 failed | 11 passed in the file).npx vitest run tests/unit/cli/tui/actions/headless-set.test.ts→ 12 passed.npx vitest run→ 8300 passed, 72 failed, 11 skipped, 7 todo (720 files: 702 passed | 15 failed | 3 skipped). Failures are pre-existing / environment (integration browser pool, SPA settle, ImportScreen TUI timing, repl e2e) — none inheadless-set.npx tsc --noEmit(npm run lint) → clean, exit 0.Checklist
fix(config): ...)CONTRIBUTING.md(issue already open; tests added; no new deps)Summary by CodeRabbit
Bug Fixes
--setso settings can be selected using either environment-style keys or settings paths.Tests