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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# Changelog

## Unreleased

### Changed

- Blocks should have key-value options separated by commas. Existing syntax remains is supported for back-compatibility. See [the documentation on Additional Options](https://tommilligan.github.io/mdbook-admonish/#additional-options) for more details ([#181](https://github.com/tommilligan/mdbook-admonish/pull/181))

### Fixed

- Titles contining `=` will now render correctly. Thanks to [@s00500](https://github.com/s00500) for the bug report! ([#181](https://github.com/tommilligan/mdbook-admonish/pull/181))

## v1.16.0

### Changed
Expand Down
1 change: 1 addition & 0 deletions book/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@

- [Overview](./overview.md)
- [Reference](./reference.md)
- [Examples](./examples.md)
15 changes: 15 additions & 0 deletions book/src/examples.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Examples

## Combining multiple custom properties

Note that the comma `,` is used to seperate custom options.

````
```admonish quote collapsible=true, title='A title that really <span style="color: #e70073">pops</span>'
To really <b><span style="color: #e70073">grab</span></b> your reader's attention.
```
````

```admonish quote collapsible=true, title='A title that really <span style="color: #e70073">pops</span>'
To really <b><span style="color: #e70073">grab</span></b> your reader's attention.
```
23 changes: 14 additions & 9 deletions book/src/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,19 @@ You can also configure the build to fail loudly, by setting `on_failure = "bail"

### Additional Options

You can pass additional options to each block. The options are structured as TOML key-value pairs.
You can pass additional options to each block. Options are given like a [TOML Inline Table](https://toml.io/en/v1.0.0#inline-table), as key-value pairs separated by commas.

`mdbook-admonish` parses options by wrapping your options in an inline table before parsing them, so please consult [The TOML Reference](https://toml.io) if you run into any syntax errors. Be aware that:

- Key-value pairs must be separated with a comma `,`
- TOML escapes must be escaped again - for instance, write `\"` as `\\"`.
- For complex strings such as HTML, you may want to use a [literal string](https://toml.io/en/v1.0.0#string) to avoid complex escape sequences

Note that some options can be passed globally, through the `default` section in `book.toml`. See the [configuration reference](./reference.md#booktoml-configuration) for more details.

#### Custom title

A custom title can be provided, contained in a double quoted TOML string.
Note that TOML escapes must be escaped again - for instance, write `\"` as `\\"`.
A custom title can be provided:

````
```admonish warning title="Data loss"
Expand Down Expand Up @@ -114,13 +119,13 @@ This will take a while, go and grab a drink of water.
Markdown and HTML can be used in the inner content, as you'd expect:

````
```admonish tip title="_Referencing_ and <i>dereferencing</i>"
```admonish tip title='_Referencing_ and <i>dereferencing</i>'
The opposite of *referencing* by using `&` is *dereferencing*, which is
accomplished with the <span style="color: hotpink">dereference operator</span>, `*`.
```
````

```admonish tip title="_Referencing_ and <i>dereferencing</i>"
```admonish tip title='_Referencing_ and <i>dereferencing</i>'
The opposite of *referencing* by using `&` is *dereferencing*, which is
accomplished with the <span style="color: hotpink">dereference operator</span>, `*`.
```
Expand Down Expand Up @@ -148,7 +153,7 @@ print "Hello, world!"
If you want to provide custom styling to a specific admonition, you can attach one or more custom classnames:

````
```admonish note class="custom-0 custom-1"
```admonish note title="Stylish", class="custom-0 custom-1"
Styled with my custom CSS class.
```
````
Expand All @@ -173,7 +178,7 @@ with an appended number if multiple blocks would have the same id.
Setting the `id` field will _ignore_ all other ids and the duplicate counter.

````
```admonish info title="My Info" id="my-special-info"
```admonish info title="My Info", id="my-special-info"
Link to this block with `#my-special-info` instead of the default `#admonition-my-info`.
```
````
Expand All @@ -183,14 +188,14 @@ Link to this block with `#my-special-info` instead of the default `#admonition-m
For a block to be initially collapsible, and then be openable, set `collapsible=true`:

````
```admonish collapsible=true
```admonish title="Sneaky", collapsible=true
Content will be hidden initially.
```
````

Will yield something like the following HTML, which you can then apply styles to:

```admonish collapsible=true
```admonish title="Sneaky", collapsible=true
Content will be hidden initially.
```

Expand Down
6 changes: 3 additions & 3 deletions integration/expected/chapter_1_main.html
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ <h1 id="chapter-1"><a class="header" href="#chapter-1">Chapter 1</a></h1>
<p>Failed with:</p>
<pre><code class="language-log">'title=&quot;' is not a valid directive or TOML key-value pair.

TOML parsing error: TOML parse error at line 1, column 8
TOML parsing error: TOML parse error at line 1, column 21
|
1 | title=&quot;
| ^
1 | config = { title=&quot; }
| ^
invalid basic string

</code></pre>
Expand Down
50 changes: 38 additions & 12 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
mod toml_wrangling;
mod v1;
mod v2;
mod v3;

/// Configuration as described by the instance of an admonition in markdown.
///
/// This structure represents the configuration the user must provide in each
/// instance.
#[derive(Debug, PartialEq)]
#[derive(Debug, PartialEq, Default)]
pub(crate) struct InstanceConfig {
pub(crate) directive: String,
pub(crate) title: Option<String>,
Expand Down Expand Up @@ -35,20 +37,29 @@ impl InstanceConfig {
/// - `Some(InstanceConfig)` if this is an `admonish` block
pub fn from_info_string(info_string: &str) -> Option<Result<Self, String>> {
let config_string = admonition_config_string(info_string)?;
Some(Self::from_admonish_config_string(config_string))
}

/// Parse an info string that is known to be for `admonish`.
fn from_admonish_config_string(config_string: &str) -> Result<Self, String> {
// If we succeed at parsing v3, return that. Otherwise hold onto the error
let config_v3_error = match v3::from_config_string(config_string) {
Ok(config) => return Ok(config),
Err(error) => error,
};

// If we succeed at parsing v2, return that. Otherwise hold onto the error
let config_v2_error = match v2::from_config_string(config_string) {
Ok(config) => return Some(Ok(config)),
Err(config) => config,
// If we succeed at parsing v2, return that
if let Ok(config) = v2::from_config_string(config_string) {
return Ok(config);
};

Some(if let Ok(config) = v1::from_config_string(config_string) {
// If we succeed at parsing v1, return that.
Ok(config)
} else {
// Otherwise return our v2 error.
Err(config_v2_error)
})
// If we succeed at parsing v1, return that.
if let Ok(config) = v1::from_config_string(config_string) {
return Ok(config);
}

// Otherwise return our v3 error.
Err(config_v3_error)
}
}

Expand Down Expand Up @@ -90,5 +101,20 @@ mod test {
collapsible: None,
}
);
// v3 syntax is supported
assert_eq!(
InstanceConfig::from_info_string(
r#"admonish title="Custom Title", type="question", id="my-id""#
)
.unwrap()
.unwrap(),
InstanceConfig {
directive: "question".to_owned(),
title: Some("Custom Title".to_owned()),
id: Some("my-id".to_owned()),
additional_classnames: Vec::new(),
collapsible: None,
}
);
}
}
44 changes: 44 additions & 0 deletions src/config/toml_wrangling.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
use once_cell::sync::Lazy;
use regex::Regex;
use serde::Deserialize;
use std::fmt::Display;

#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
pub(crate) struct UserInput {
#[serde(default)]
pub r#type: Option<String>,
#[serde(default)]
pub title: Option<String>,
#[serde(default)]
pub id: Option<String>,
#[serde(default)]
pub class: Option<String>,
#[serde(default)]
pub collapsible: Option<bool>,
}

impl UserInput {
pub fn classnames(&self) -> Vec<String> {
self.class
.as_ref()
.map(|class| {
class
.split(' ')
.filter(|classname| !classname.is_empty())
.map(|classname| classname.to_owned())
.collect()
})
.unwrap_or_default()
}
}

pub(crate) static RX_DIRECTIVE: Lazy<Regex> =
Lazy::new(|| Regex::new(r#"^[A-Za-z0-9_-]+$"#).expect("directive regex"));

pub(crate) fn format_toml_parsing_error(error: impl Display) -> String {
format!("TOML parsing error: {error}")
}

pub(crate) fn format_invalid_directive(directive: &str, original_error: impl Display) -> String {
format!("'{directive}' is not a valid directive or TOML key-value pair.\n\n{original_error}")
}
41 changes: 15 additions & 26 deletions src/config/v2.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,9 @@
use super::toml_wrangling::{
format_invalid_directive, format_toml_parsing_error, UserInput, RX_DIRECTIVE,
};
use super::InstanceConfig;
use once_cell::sync::Lazy;
use regex::Regex;
use serde::Deserialize;

#[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
struct UserInput {
#[serde(default)]
r#type: Option<String>,
#[serde(default)]
title: Option<String>,
#[serde(default)]
id: Option<String>,
#[serde(default)]
class: Option<String>,
#[serde(default)]
collapsible: Option<bool>,
}

/// Transform our config string into valid toml
fn bare_key_value_pairs_to_toml(pairs: &str) -> String {
Expand All @@ -39,18 +27,26 @@ fn bare_key_value_pairs_to_toml(pairs: &str) -> String {
.into_owned()
}

fn user_input_from_config_toml(config_toml: &str) -> Result<UserInput, String> {
toml::from_str(config_toml).map_err(format_toml_parsing_error)
}

/// Parse and return the config assuming v2 format.
///
/// Note that if an error occurs, a parsed struct that can be returned to
/// show the error message will be returned.
///
/// The basic idea here is to accept space separated key-value pairs, break them
/// onto separate lines, and then parse them as a TOML document.
/// This breaks when values contain a literal '=' sign, for which v3 syntax should be used.
pub(crate) fn from_config_string(config_string: &str) -> Result<InstanceConfig, String> {
let config_toml = bare_key_value_pairs_to_toml(config_string);
let config_toml = config_toml.trim();

let config: UserInput = match toml::from_str(config_toml) {
Ok(config) => config,
Err(error) => {
let original_error = format!("TOML parsing error: {error}");
let original_error = format_toml_parsing_error(error);

// For ergonomic reasons, we allow users to specify the directive without
// a key. So if parsing fails initially, take the first line,
Expand All @@ -60,19 +56,11 @@ pub(crate) fn from_config_string(config_string: &str) -> Result<InstanceConfig,
None => (config_toml, ""),
};

static RX_DIRECTIVE: Lazy<Regex> =
Lazy::new(|| Regex::new(r#"^[A-Za-z0-9_-]+$"#).expect("directive regex"));

if !RX_DIRECTIVE.is_match(directive) {
return Err(format!("'{directive}' is not a valid directive or TOML key-value pair.\n\n{original_error}"));
return Err(format_invalid_directive(directive, original_error));
}

let mut config: UserInput = match toml::from_str(config_toml) {
Ok(config) => config,
Err(error) => {
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

config.r#type = Some(directive.to_owned());
config
}
Expand Down Expand Up @@ -188,6 +176,7 @@ mod test {
)?;
// Directive after toml config is an error
assert!(from_config_string(r#"title="Information" info"#).is_err());

Ok(())
}

Expand Down
Loading
Loading