Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: support toml values with equal sign #181

Merged
merged 1 commit into from
May 24, 2024

Conversation

tommilligan
Copy link
Owner

Closes #179

The v2 config format is not suitable for parsing complex toml values, specifically containing = values. This is due to the pre-parsing wrangling to marshal key-value pairs into a valid TOML format using = as a marker for TOML keys, when in fact it can appear in values too.

To fix this, introduce the new v3 config format which uses the TOML Inline Table syntax to support this natively.

This PR adds the config back-compatibly, falling back to v2 (and v1) if parsing with the new format fails. It also updates all docs/examples to use the new comma separated format where appropriate.

@tommilligan tommilligan force-pushed the fix-breaking-on-equals-in-value branch from 0581803 to ffb819c Compare May 24, 2024 12:00
@tommilligan tommilligan merged commit 9df896c into main May 24, 2024
6 checks passed
return Err(format!("TOML parsing error: {error}"));
}
};
let mut config = user_input_from_config_toml(dbg!(config_toml))?;
Copy link

Choose a reason for hiding this comment

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

Hey @tommilligan, seems like this line now causes quite some prints during the build that are not really relevant. Very minor issue, but maybe that was not intended ?

Looks like this:
.cargo/registry/src/index.crates.io-6f17d22bba15001f/mdbook-admonish-1.17.0/src/config/v2.rs:63:58] config_toml = "collapsible=true \ntitle=\"Display value (without control)\""

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thanks, yes this was indeed unintentional. Opened #186 to resolve

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.

Using other markdown or html in titles
2 participants