Skip to content

fix(config): ignore unknown ini sections instead of panicking - #74

Merged
chess-seventh merged 3 commits into
masterfrom
feat/tolerant-config
Jul 30, 2026
Merged

chess-seventh merged 3 commits into
masterfrom
feat/tolerant-config

Conversation

@chess-seventh

Copy link
Copy Markdown
Owner

One ini file is shared by every checkout, so a section written for a newer release reached binaries that predate it: adding [exclude] made every 4.14.x pin panic on the section count, and those repos silently stopped journalling commits.

  • [obsidian] and [templates] stay required; missing either is still fatal
  • any other section is ignored with a warning
  • tests: flipped the three-sections test, added one for a future-release section
  • README states the forward-compatibility contract

Refs: L66

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.02326% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/config.rs 93.02% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@chess-seventh
chess-seventh force-pushed the feat/tolerant-config branch from e73935a to ec73f5d Compare July 29, 2026 22:48
One ini file is shared by every checkout on the machine, so a section written
for a newer release reached binaries that predate it: adding [exclude] made
every checkout older than 4.17.0 panic on the section count and stop
journalling its commits. Strict section validation turns any future config
addition into the same outage.

- keep [obsidian] and [templates] required; missing either is still fatal
- ignore any other section instead of panicking
- report the ignored section on stderr as well as through log::warn, because
  the git hook runs without RUST_LOG and env_logger would swallow the warning;
  an [excludes] typo must not silently journal the repos you excluded
- cover that report with integration tests that run the binary: removing the
  stderr line fails them, which the unit tests alone did not
- hold the known-section list in one const, so a section added to the dispatch
  cannot be applied and reported as unrecognised at the same time
- correct the set_obsidian_vars rustdoc and its call-site comment, which both
  still promised that an unknown section was rejected
- tests: flip the three-sections case, add a section from a future release and
  the [obsidian]-without-[templates] mirror, and rename three tests whose
  names encoded the pre-4.17 contract
- state the forward-compatibility contract in the README config section

Co-Authored-By: Vulcan <chess7th@pm.me>
The panic on a missing root_path_dir blamed commit_path, sending anyone
debugging a broken config at the wrong line of their ini.

- report root_path_dir in its own expect message

Co-Authored-By: Vulcan <chess7th@pm.me>
@chess-seventh
chess-seventh force-pushed the feat/tolerant-config branch from ec73f5d to 420dc7b Compare July 29, 2026 23:13
The review found the new guards passed for weak reasons: the stderr test could
have passed vacuously, and the corrected panic message was asserted only by its
prefix, so reverting it went unnoticed.

- pin the root_path_dir expect message in should_panic, not just "Could not
  get"; reverting the message now fails the test
- assert the run reaches past config loading before asserting the silent case,
  so a config that never loads cannot pass it
- keep the test vault inside the temp dir instead of a shared /tmp path
- drop two stale wordings: an assert message about a section count that is no
  longer validated, and a comment naming 4.14.x where any pre-4.17.0 binary
  is affected

Co-Authored-By: Vulcan <chess7th@pm.me>
@chess-seventh
chess-seventh merged commit a6f02c5 into master Jul 30, 2026
6 checks passed
@chess-seventh
chess-seventh deleted the feat/tolerant-config branch July 30, 2026 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant