fix: report a broken config file instead of silently using defaults - #125
Merged
Conversation
A parse failure was swallowed by unwrap_or_else, so a typo made every customization vanish with no message and a zero exit. ensure_file_exists already seeds a missing file with the defaults, so a failure here is real. Closes ynqa#117
ynqa
approved these changes
Aug 19, 2026
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
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.
Closes #117.
What's broken
A config file that fails to parse is discarded silently. No message, no warning, exit 0, and every customization is gone with no indication why.
A config that is valid TOML but incomplete does it too, which is the easier way to hit this by accident:
The error was there all along, just thrown away:
The cause
unwrap_or_else(|_e| ...)inmainswallows the error and substitutesDEFAULT_CONFIG.The fix
Propagate it. Now:
Why failing is right here, rather than warning and continuing
ensure_file_existsalready creates the file with the defaults when it is absent, for both the explicit--configpath and the default location. So "no config" is handled before this point and never reaches it.That means anything failing here is a file that exists and is broken, which is worth stopping for. A user who passes
--configand gets defaults instead has silently lost their settings.Tell me if you would rather warn and continue and I will change it.
Verification
A valid config still loads: running with
default.tomlgets past config loading to terminal setup as before.cargo fmt --check,cargo clippy --all-targets -- -D warnings,cargo testandcargo buildall pass, matching the five steps inci.yml.No test added, because the repo currently has no test target at all and
cargo testreports 0 tests. Adding a harness felt like a bigger change than the fix. Happy to add one if you want the infrastructure.