feat: add --with-config-override CLI flag#1492
Conversation
|
🎉 Thank you for your first contribution to superfile! We’re really excited to have you here 🙌 A maintainer might ask you to make a few changes before we can merge this PR. 👉 Please also take a moment to review our Contribution Guide If you have any questions, feel free to open a Discussion or just ask in the comments! |
github-actions
Bot
added
test related
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds a repeatable ChangesCLI Config Override Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae175affb7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return fmt.Errorf("invalid config override %q: expected format 'key=value'", override) | ||
| } | ||
| key = strings.TrimSpace(key) | ||
| rawValue = strings.TrimSpace(rawValue) |
There was a problem hiding this comment.
For string overrides where whitespace is the intended value, this trims the value before assignment; for example, the default config documents Use ' ' for borderless in src/superfile_config/config.toml:141, but --wco 'border_top= ' becomes an empty string here and then fails ValidateConfig because the border is no longer one cell wide. The key can be trimmed, but the raw value should remain intact for string fields so CLI overrides can represent the same values as config.toml.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/internal/common/config_override.go (1)
72-106: 💤 Low valueMinor comment precision: nil vs empty slice.
The comment on line 93 states "An empty string yields an empty slice rather than [""]", but when
raw == "", the code leavespartsasnil(the zero value of[]string), not an empty slice[]string{}. While nil and empty slices are functionally equivalent in Go (both have length 0, range the same way, and marshal identically), the comment could be more precise.This is not a functional issue—just a documentation nit.
📝 Optional precision improvement
- // Comma separated list, mirroring how TOML arrays of strings are written - // inline. An empty string yields an empty slice rather than [""]. + // Comma separated list, mirroring how TOML arrays of strings are written + // inline. An empty string yields a nil slice rather than [""].Or, if you prefer truly empty slices:
- var parts []string + parts := []string{} if raw != "" { parts = strings.Split(raw, ",")🤖 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 `@src/internal/common/config_override.go` around lines 72 - 106, The comment in the setFieldFromString function that states "An empty string yields an empty slice rather than [""]" is inaccurate. When raw is an empty string, the code leaves parts as nil (the zero value of []string), not an empty slice. Update this comment to correctly reflect that an empty string results in a nil slice, not an empty slice, since the variable parts is never reassigned from its zero value when the raw string is empty.
🤖 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.
Nitpick comments:
In `@src/internal/common/config_override.go`:
- Around line 72-106: The comment in the setFieldFromString function that states
"An empty string yields an empty slice rather than [""]" is inaccurate. When raw
is an empty string, the code leaves parts as nil (the zero value of []string),
not an empty slice. Update this comment to correctly reflect that an empty
string results in a nil slice, not an empty slice, since the variable parts is
never reassigned from its zero value when the raw string is empty.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b305e6b-c72d-4a92-a5cb-f0b0cf91a6e8
📒 Files selected for processing (5)
src/cmd/main.gosrc/config/fixed_variable.gosrc/internal/common/config_override.gosrc/internal/common/config_override_test.gosrc/internal/common/load_config.go
youdie006
force-pushed
the
feat/config-override-flag
branch
from
ae175af to
001253a
Compare
Add a repeatable --with-config-override 'key=value' flag (alias --wco) that overrides individual config.toml values from the CLI (yorukot#1272). ApplyConfigOverrides maps each TOML key to its ConfigType field via the toml tags, splits on the first =, and coerces the value by field kind (string/bool/int/[]string), running after the config decodes and before ValidateConfig. Unknown keys, missing =, and unparseable values each return a clear error naming the key. Fixes yorukot#1272
youdie006
force-pushed
the
feat/config-override-flag
branch
from
001253a to
a67a1a7
Compare
1 participant
Description
Adds a repeatable
--with-config-override 'key=value'flag (alias--wco) that overrides individualconfig.tomlvalues from the CLI, as requested in #1272 (modeled on rovr's--with/--without).ApplyConfigOverrides(newsrc/internal/common/config_override.go) maps each TOML key to itsConfigTypefield via thetoml:"..."tags — the same reflection approachLoadTomlFileuses — splits on the first=(so values may contain=), and coerces the value by field kind (string / bool / int / []string). It runs after the config file decodes and beforeValidateConfig, so overrides are validated like the rest of the config. Unknown keys, a missing=, and unparseable values each return a clear error naming the key.Example:
spf --with-config-override 'debug=true'(repeatable:--wco 'a=1' --wco 'b=2').Related Issues
Fixes #1272
Screenshots
CLI flag + config behavior (no UI change). Verified manually:
--with-config-override 'debug=true'launched the TUI and produced DEBUG-level log entries (none without the override); error paths exit before the TUI with a clear message; the flag appears in--help.Checklist
go fmt(gofmt clean)golangci-lint run— not available in my environment;go vet,go build, andgo test ./...pass (only pre-existing env failures: missingexiftool/zoxidebinaries)Summary by CodeRabbit
New Features
--with-config-override(alias--wco) to override configuration values usingkey=valuesyntax.Bug Fixes
Tests