Skip to content

fix: fields missing from config.toml parsing logic#1843

Merged
mre merged 3 commits intolycheeverse:masterfrom
rina-forks:fix-fold-in
Sep 9, 2025
Merged

fix: fields missing from config.toml parsing logic#1843
mre merged 3 commits intolycheeverse:masterfrom
rina-forks:fix-fold-in

Conversation

@katrinafyi
Copy link
Member

@katrinafyi katrinafyi commented Sep 7, 2025

the fold_in! macro relies on the listed fields in order to know which fields to merge from the toml file. if fields were missing from this list, as they were previously, then the toml file values for those fields would be silently ignored.

this pr adds those missing fields, except for header and github_token which are handled separately. this also sorts the field list alphabetically.

it would be nice if there was a "kitchen sink" toml file which had every option set. maybe this could be auto-generated (related to #1768).

command to list all config fields:

grep --after-context=10000 'pub(crate) struct Config' \
  lychee-bin/src/options.rs | grep '    pub(crate) ' \
  | cut -d' ' -f6 | grep : | sort

getting all fields within fold_in! was done by vim ya{

the `fold_in!` macro relies on the listed fields in order to know which
fields to merge from the toml file.

if fields were missing from this list, as they were previously, then
the toml file values for those fields would be silently ignored.

this also sorts the field list alphabetically.

it would be nice if there was a "kitchen sink" toml file which had every
option set. maybe this could be auto-generated.

command to list all config fields:
```
grep --after-context=10000 'pub(crate) struct Config' \
  lychee-bin/src/options.rs | grep '    pub(crate) ' \
  | cut -d' ' -f6 | grep : | sort
```
getting all fields within `fold_in!` was done by vim `ya{`
@katrinafyi katrinafyi changed the title fix: add missing fields to fold_in macro fix: add missing fields to config.toml parsing logic Sep 8, 2025
@katrinafyi katrinafyi changed the title fix: add missing fields to config.toml parsing logic fix: fields missing from config.toml parsing logic Sep 8, 2025
@mre
Copy link
Member

mre commented Sep 9, 2025

Nice. Yeah, that sort of problem keeps resurfacing. Would be nice to get a compile-time error if we forgot to fold a field.

Or maybe we can look into alternatives like https://github.com/gibfahn/clap_config or similar, but that would be a bigger change. It might not be so easy because we also want to skip a few fields as you mentioned.

it would be nice if there was a "kitchen sink" toml file which had every option set. maybe this could be auto-generated (related to #1768).

The closest thing to that is https://github.com/lycheeverse/lychee/blob/master/fixtures/configs/smoketest.toml but I'm sure there are missing options by now. Might be worth investigating.

Either way, thanks for fixing.

@mre mre merged commit 293f455 into lycheeverse:master Sep 9, 2025
6 checks passed
@mre mre mentioned this pull request Sep 9, 2025
This was referenced Oct 21, 2025
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.

2 participants