Skip to content

Conversation

@szabgab
Copy link
Contributor

@szabgab szabgab commented Mar 6, 2025

To avoid typos and make the config reading stricter.

See #2493

To avoid typos and make the config reading stricter.
@rustbot rustbot added the S-waiting-on-review Status: waiting on a review label Mar 6, 2025
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! However, I don't think we can go directly to making this an error. See #1595 for a discussion about how this can be approached and some of the complexities.

Comment on lines 897 to 906
#[test]
#[should_panic(expected = "Invalid configuration file")]
fn fail_on_invalid_config_field() {
let src = r#"
[book]
password = Secret
"#;

Config::from_str(src).unwrap();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this test is actually checking the change in this PR. It seems to be checking that the TOML syntax is invalid.

@szabgab
Copy link
Contributor Author

szabgab commented Mar 6, 2025

Oh, right, I am still not used to the requirement of quotes in toml files.
I think the test now check what I intended.

I'll now look into #1595

@ehuss ehuss added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: waiting on a review labels Mar 21, 2025
@ehuss
Copy link
Contributor

ehuss commented Aug 12, 2025

Closing as this is now pulled into #2787. Thanks!

@ehuss ehuss closed this Aug 12, 2025
@szabgab szabgab deleted the strict-configuration-fields branch September 5, 2025 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants