bin: better error handling

.github/workflows/check.yml

@@ -70,7 +70,7 @@ jobs:
         rust:
           - stable
           - beta
-          - 1.64.0
+          - 1.60.0
         experimental:
           - false
         # Run a canary test on nightly that's allowed to fail

CHANGELOG.md

@@ -1,26 +1,7 @@
-# Changelog
+## Changelog
 
 ## Unreleased
 
-## 1.9.0
-
-### Changed
-
-- Styles updated to `^2.0.1`. Run `mdbook-admonish install` to update.
-- MSRV (minimum supported rust version) is now 1.64.0 for clap v4 ([#79](https://github.com/tommilligan/mdbook-admonish/pull/79))
-- More verbose error messages for invalid TOML configurations ([#79](https://github.com/tommilligan/mdbook-admonish/pull/79))
-
-### Added
-
-- User can set book-wide default for title and collapsible properties ([#84](https://github.com/tommilligan/mdbook-admonish/pull/84)), thanks to [@ShaunSHamilton](https://github.com/ShaunSHamilton)
-
-### Fixed
-
-- Custom installation and CSS directories are now normalized ([#49](https://github.com/tommilligan/mdbook-admonish/pull/49))
-- Fix title bars with no text rendering badly ([#83](https://github.com/tommilligan/mdbook-admonish/pull/83)), thanks to [@ShaunSHamilton](https://github.com/ShaunSHamilton)
-- Better error message display on crash ([#48](https://github.com/tommilligan/mdbook-admonish/pull/48))
-- Better support for commonmark code fence syntax ([#88](https://github.com/tommilligan/mdbook-admonish/pull/88), [#89](https://github.com/tommilligan/mdbook-admonish/pull/89))
-
 ## 1.8.0
 
 ### Changed
@@ -36,7 +17,7 @@
 ### Added
 
 - Support key/value configuration ([#24](https://github.com/tommilligan/mdbook-admonish/pull/24), thanks [@gggto](https://github.com/gggto) and [@schungx](https://github.com/schungx) for design input)
-- Support collapsible admonition bodies ([#26](https://github.com/tommilligan/mdbook-admonish/pull/26), thanks [@gggto](https://github.com/gggto) for the suggestion and implementation!)
+- Support collapsiable admonition bodies ([#26](https://github.com/tommilligan/mdbook-admonish/pull/26), thanks [@gggto](https://github.com/gggto) for the suggestion and implementation!)
 - Make anchor links hoverable ([#27](https://github.com/tommilligan/mdbook-admonish/pull/27))
 - Better handling for misconfigured admonitions ([#25](https://github.com/tommilligan/mdbook-admonish/pull/25))
   - Nicer in-book error messages

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "mdbook-admonish"
-version = "1.9.0"
+version = "1.8.0"
 edition = "2021"
 
 authors = ["Tom Milligan <code@tommilligan.net>"]
@@ -24,7 +24,7 @@ path = "src/lib.rs"
 [dependencies]
 anyhow = "1.0.65"
 clap = { version = "4", default_features = false, features = ["std", "derive"], optional = true }
-env_logger = { version = "0.10", default_features = false, optional = true }
+env_logger = { version = "0.9.1", default_features = false, optional = true }
 log = { version = "0.4.17", optional = true }
 mdbook = "0.4.21"
 once_cell = "1.15.0"
@@ -33,8 +33,8 @@ regex = "1.6.0"
 semver = "1.0.14"
 serde = { version = "1.0.145", features = ["derive"] }
 serde_json = "1.0.85"
-toml = "0.7.3"
-toml_edit = { version = "0.19.8", optional = true }
+toml = "0.5.9"
+toml_edit = { version = "0.15.0", optional = true }
 
 [dev-dependencies]
 pretty_assertions = "1.3.0"

README.md

@@ -136,17 +136,6 @@ on_failure = "bail"
 
 This may be useful for non-interative workflows.
 
-### Process included files
-
-You can ensure that content inlined with `{{#include}}` is also processed by [setting the `after` option](https://rust-lang.github.io/mdBook/format/configuration/preprocessors.html#require-a-certain-order):
-
-```toml
-[preprocessor.admonish]
-after = ["links"]
-```
-
-This will expand `include` directives, before expanding `admonish` blocks.
-
 ### Semantic Versioning
 
 Guarantees provided are as follows:

book/book.toml

@@ -14,4 +14,4 @@ assets_version = "2.0.0" # do not edit: managed by `mdbook-admonish install`
 [output]
 
 [output.html]
-additional-css = ["./mdbook-admonish.css"]
+additional-css = ["././mdbook-admonish.css"]

compile_assets/scss/admonition.scss

@@ -145,7 +145,6 @@ a.admonition-anchor-link {
 // Admonition title
 :is(.admonition-title, summary) {
   position: relative;
-  min-height: 4rem;
   margin-block: 0;
   margin-inline: -1.6rem -1.2rem;
   padding-block: 0.8rem;

integration/book.toml

@@ -9,10 +9,9 @@ title = "mdbook-admonish-integration"
 
 [preprocessor.admonish]
 command = "mdbook-admonish"
-assets_version = "2.0.1" # do not edit: managed by `mdbook-admonish install`
-after = ["links"]
+assets_version = "2.0.0" # do not edit: managed by `mdbook-admonish install`
 
 [output]
 
 [output.html]
-additional-css = ["./mdbook-admonish.css"]
+additional-css = ["././mdbook-admonish.css"]

integration/expected/book.toml

@@ -9,10 +9,9 @@ title = "mdbook-admonish-integration"
 
 [preprocessor.admonish]
 command = "mdbook-admonish"
-assets_version = "2.0.1" # do not edit: managed by `mdbook-admonish install`
-after = ["links"]
+assets_version = "2.0.0" # do not edit: managed by `mdbook-admonish install`
 
 [output]
 
 [output.html]
-additional-css = ["./mdbook-admonish.css"]
+additional-css = ["././mdbook-admonish.css"]

integration/expected/chapter_1_main.html

@@ -29,14 +29,7 @@
 <p><a class="admonition-anchor-link" href="#admonition-error-rendering-admonishment"></a></p>
 </div>
 <div>
-<p>Failed with:</p>
-<pre><code>TOML parsing error: TOML parse error at line 1, column 8
-  |
-1 | title=&quot;
-  |        ^
-invalid basic string
-
-</code></pre>
+<p>Failed with: TOML parsing error: unterminated string at line 1 column 7</p>
 <p>Original markdown input:</p>
 <pre><code>```admonish title=&quot;
 No title, only body
@@ -53,23 +46,4 @@ No title, only body
 <p>Hidden on load</p>
 </div>
 </details>
-<div id="admonition-warning" class="admonition warning">
-<div class="admonition-title">
-<p>Warning</p>
-<p><a class="admonition-anchor-link" href="#admonition-warning"></a></p>
-</div>
-<div>
-<p>This is a commonly shared warning!</p>
-</div>
-</div>
-<div id="admonition-note-2" class="admonition note">
-<div class="admonition-title">
-<p>Note</p>
-<p><a class="admonition-anchor-link" href="#admonition-note-2"></a></p>
-</div>
-<div>
-<pre><code class="language-bash">Nested code block
-</code></pre>
-</div>
-</div>
 

integration/src/chapter_1.md

@@ -21,11 +21,3 @@ No title, only body
 ```admonish collapsible=true
 Hidden on load
 ```
-
-{{#include common_warning.md}}
-
-````admonish
-```bash
-Nested code block
-```
-````

integration/src/common_warning.md

@@ -1,3 +0,0 @@
-```admonish warning
-This is a commonly shared warning!
-```

qvet.yml

@@ -1,14 +0,0 @@
-action:
-  ready:
-    type: link
-    name: "Push New Version Tag"
-    url: "https://github.com/tommilligan/mdbook-admonish"
-
-commit:
-  ignore:
-    merges: true
-
-release:
-  identifiers:
-    - type: tag
-      pattern: "^v[0-9]"

scripts/check

@@ -11,6 +11,18 @@ function eprintln() {
 eprintln "Formatting sources"
 cargo fmt -- --check
 
+# Known issues:
+# - RUSTSEC-2020-0071 known unlikely segfault in `time`
+# - RUSTSEC-2020-0016 `net2` is unmaintained
+# - RUSTSEC-2020-0159 known unlikely segfault in `chrono`
+# - RUSTSEC-2021-0145 known unmaintained atty transitive dep
+eprintln "Auditing dependencies"
+cargo audit --deny warnings \
+  --ignore RUSTSEC-2020-0071 \
+  --ignore RUSTSEC-2020-0016 \
+  --ignore RUSTSEC-2021-0145 \
+  --ignore RUSTSEC-2020-0159
+
 eprintln "Linting sources"
 cargo clippy --all-targets -- -D warnings
 

scripts/install

@@ -10,4 +10,8 @@ cd "$(dirname "$0")"/..
 
 rustup component add rustfmt clippy
 
+if ! cargo audit --version; then
+  cargo install cargo-audit --force
+fi
+
 ./scripts/install-mdbook

src/bin/assets/VERSION

@@ -1 +1 @@
-2.0.1
+2.0.0

src/bin/assets/mdbook-admonish.css

@@ -77,7 +77,6 @@ a.admonition-anchor-link::before {
 
 :is(.admonition-title, summary) {
   position: relative;
-  min-height: 4rem;
   margin-block: 0;
   margin-inline: -1.6rem -1.2rem;
   padding-block: 0.8rem;

src/bin/mdbook-admonish.rs

@@ -144,10 +144,7 @@ mod install {
 
         let mut additional_css = additional_css(&mut doc);
         for (name, content) in ADMONISH_CSS_FILES {
-            let filepath = proj_dir.join(css_dir.clone()).join(name);
-            // Normalize path to remove no-op components
-            // https://github.com/tommilligan/mdbook-admonish/issues/47
-            let filepath: PathBuf = filepath.components().collect();
+            let filepath = proj_dir.join(&css_dir).join(name);
             let filepath_str = filepath.to_str().context("non-utf8 filepath")?;
 
             if let Ok(ref mut additional_css) = additional_css {

src/config/mod.rs

@@ -1,16 +1,15 @@
+use crate::types::Directive;
+use std::str::FromStr;
+
 mod v1;
 mod v2;
 
-/// 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)]
-pub(crate) struct InstanceConfig {
-    pub(crate) directive: String,
-    pub(crate) title: Option<String>,
-    pub(crate) additional_classnames: Vec<String>,
-    pub(crate) collapsible: Option<bool>,
+pub(crate) struct AdmonitionInfoRaw {
+    directive: String,
+    title: Option<String>,
+    additional_classnames: Vec<String>,
+    collapsible: bool,
 }
 
 /// Extract the remaining info string, if this is an admonition block.
@@ -28,10 +27,10 @@ fn admonition_config_string(info_string: &str) -> Option<&str> {
     }
 }
 
-impl InstanceConfig {
+impl AdmonitionInfoRaw {
     /// Returns:
     /// - `None` if this is not an `admonish` block.
-    /// - `Some(InstanceConfig)` if this is an `admonish` block
+    /// - `Some(AdmonitionInfoRaw)` 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)?;
 
@@ -41,13 +40,65 @@ impl InstanceConfig {
             Err(config) => 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)
-        })
+        Some(
+            if let Ok(info_raw) = v1::from_config_string(config_string) {
+                // If we succeed at parsing v1, return that.
+                Ok(info_raw)
+            } else {
+                // Otherwise return our v2 error.
+                Err(config_v2_error)
+            },
+        )
+    }
+}
+
+#[derive(Debug, PartialEq)]
+pub(crate) struct AdmonitionInfo {
+    pub directive: Directive,
+    pub title: Option<String>,
+    pub additional_classnames: Vec<String>,
+    pub collapsible: bool,
+}
+
+impl AdmonitionInfo {
+    pub fn from_info_string(info_string: &str) -> Option<Result<Self, String>> {
+        AdmonitionInfoRaw::from_info_string(info_string).map(|result| result.map(Into::into))
+    }
+}
+
+impl From<AdmonitionInfoRaw> for AdmonitionInfo {
+    fn from(other: AdmonitionInfoRaw) -> Self {
+        let AdmonitionInfoRaw {
+            directive: raw_directive,
+            title,
+            additional_classnames,
+            collapsible,
+        } = other;
+        let (directive, title) = match (Directive::from_str(&raw_directive), title) {
+            (Ok(directive), None) => (directive, ucfirst(&raw_directive)),
+            (Err(_), None) => (Directive::Note, "Note".to_owned()),
+            (Ok(directive), Some(title)) => (directive, title),
+            (Err(_), Some(title)) => (Directive::Note, title),
+        };
+        // If the user explicitly gave no title, then disable the title bar
+        let title = if title.is_empty() { None } else { Some(title) };
+        Self {
+            directive,
+            title,
+            additional_classnames,
+            collapsible,
+        }
+    }
+}
+
+/// Make the first letter of `input` upppercase.
+///
+/// source: https://stackoverflow.com/a/38406885
+fn ucfirst(input: &str) -> String {
+    let mut chars = input.chars();
+    match chars.next() {
+        None => String::new(),
+        Some(f) => f.to_uppercase().collect::<String>() + chars.as_str(),
     }
 }
 
@@ -59,30 +110,48 @@ mod test {
     #[test]
     fn test_from_info_string() {
         // Not admonition blocks
-        assert_eq!(InstanceConfig::from_info_string(""), None);
-        assert_eq!(InstanceConfig::from_info_string("adm"), None);
+        assert_eq!(AdmonitionInfoRaw::from_info_string(""), None);
+        assert_eq!(AdmonitionInfoRaw::from_info_string("adm"), None);
         // v1 syntax is supported back compatibly
         assert_eq!(
-            InstanceConfig::from_info_string("admonish note.additional-classname")
+            AdmonitionInfoRaw::from_info_string("admonish note.additional-classname")
                 .unwrap()
                 .unwrap(),
-            InstanceConfig {
+            AdmonitionInfoRaw {
                 directive: "note".to_owned(),
                 title: None,
                 additional_classnames: vec!["additional-classname".to_owned()],
-                collapsible: None,
+                collapsible: false,
             }
         );
         // v2 syntax is supported
         assert_eq!(
-            InstanceConfig::from_info_string(r#"admonish title="Custom Title" type="question""#)
+            AdmonitionInfoRaw::from_info_string(r#"admonish title="Custom Title" type="question""#)
                 .unwrap()
                 .unwrap(),
-            InstanceConfig {
+            AdmonitionInfoRaw {
                 directive: "question".to_owned(),
                 title: Some("Custom Title".to_owned()),
                 additional_classnames: Vec::new(),
-                collapsible: None,
+                collapsible: false,
+            }
+        );
+    }
+
+    #[test]
+    fn test_admonition_info_from_raw() {
+        assert_eq!(
+            AdmonitionInfo::from(AdmonitionInfoRaw {
+                directive: " ".to_owned(),
+                title: None,
+                additional_classnames: Vec::new(),
+                collapsible: false,
+            }),
+            AdmonitionInfo {
+                directive: Directive::Note,
+                title: Some("Note".to_owned()),
+                additional_classnames: Vec::new(),
+                collapsible: false,
             }
         );
     }

src/config/v1.rs

@@ -1,8 +1,8 @@
-use super::InstanceConfig;
+use super::AdmonitionInfoRaw;
 use once_cell::sync::Lazy;
 use regex::Regex;
 
-pub(crate) fn from_config_string(config_string: &str) -> Result<InstanceConfig, String> {
+pub(crate) fn from_config_string(config_string: &str) -> Result<AdmonitionInfoRaw, String> {
     let config_string = config_string.trim();
 
     static RX_CONFIG_STRING_V1: Lazy<Regex> = Lazy::new(|| {
@@ -49,11 +49,11 @@ pub(crate) fn from_config_string(config_string: &str) -> Result<InstanceConfig,
         ),
     };
 
-    Ok(InstanceConfig {
+    Ok(AdmonitionInfoRaw {
         directive: directive.to_owned(),
         title,
         additional_classnames,
-        collapsible: None,
+        collapsible: false,
     })
 }
 
@@ -66,47 +66,47 @@ mod test {
     fn test_from_config_string() {
         assert_eq!(
             from_config_string("").unwrap(),
-            InstanceConfig {
+            AdmonitionInfoRaw {
                 directive: "".to_owned(),
                 title: None,
                 additional_classnames: Vec::new(),
-                collapsible: None,
+                collapsible: false,
             }
         );
         assert_eq!(
             from_config_string(" ").unwrap(),
-            InstanceConfig {
+            AdmonitionInfoRaw {
                 directive: "".to_owned(),
                 title: None,
                 additional_classnames: Vec::new(),
-                collapsible: None,
+                collapsible: false,
             }
         );
         assert_eq!(
             from_config_string("unknown").unwrap(),
-            InstanceConfig {
+            AdmonitionInfoRaw {
                 directive: "unknown".to_owned(),
                 title: None,
                 additional_classnames: Vec::new(),
-                collapsible: None,
+                collapsible: false,
             }
         );
         assert_eq!(
             from_config_string("note").unwrap(),
-            InstanceConfig {
+            AdmonitionInfoRaw {
                 directive: "note".to_owned(),
                 title: None,
                 additional_classnames: Vec::new(),
-                collapsible: None,
+                collapsible: false,
             }
         );
         assert_eq!(
             from_config_string("note.additional-classname").unwrap(),
-            InstanceConfig {
+            AdmonitionInfoRaw {
                 directive: "note".to_owned(),
                 title: None,
                 additional_classnames: vec!["additional-classname".to_owned()],
-                collapsible: None,
+                collapsible: false,
             }
         );
     }

src/config/v2.rs

@@ -1,10 +1,10 @@
-use super::InstanceConfig;
+use super::AdmonitionInfoRaw;
 use once_cell::sync::Lazy;
 use regex::Regex;
 use serde::Deserialize;
 
 #[derive(Debug, Clone, PartialEq, Eq, Deserialize)]
-struct UserInput {
+struct AdmonitionInfoConfig {
     #[serde(default)]
     r#type: Option<String>,
     #[serde(default)]
@@ -12,7 +12,7 @@ struct UserInput {
     #[serde(default)]
     class: Option<String>,
     #[serde(default)]
-    collapsible: Option<bool>,
+    collapsible: bool,
 }
 
 /// Transform our config string into valid toml
@@ -43,11 +43,11 @@ fn bare_key_value_pairs_to_toml(pairs: &str) -> String {
 ///
 /// Note that if an error occurs, a parsed struct that can be returned to
 /// show the error message will be returned.
-pub(crate) fn from_config_string(config_string: &str) -> Result<InstanceConfig, String> {
+pub(crate) fn from_config_string(config_string: &str) -> Result<AdmonitionInfoRaw, 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) {
+    let config: AdmonitionInfoConfig = match toml::from_str(config_toml) {
         Ok(config) => config,
         Err(error) => {
             let original_error = Err(format!("TOML parsing error: {error}"));
@@ -67,7 +67,7 @@ pub(crate) fn from_config_string(config_string: &str) -> Result<InstanceConfig,
                 return original_error;
             }
 
-            let mut config: UserInput = match toml::from_str(config_toml) {
+            let mut config: AdmonitionInfoConfig = match toml::from_str(config_toml) {
                 Ok(config) => config,
                 Err(_) => return original_error,
             };
@@ -85,7 +85,7 @@ pub(crate) fn from_config_string(config_string: &str) -> Result<InstanceConfig,
                 .collect()
         })
         .unwrap_or_default();
-    Ok(InstanceConfig {
+    Ok(AdmonitionInfoRaw {
         directive: config.r#type.unwrap_or_default(),
         title: config.title,
         additional_classnames,
@@ -102,62 +102,60 @@ mod test {
     fn test_from_config_string_v2() {
         assert_eq!(
             from_config_string("").unwrap(),
-            InstanceConfig {
+            AdmonitionInfoRaw {
                 directive: "".to_owned(),
                 title: None,
                 additional_classnames: Vec::new(),
-                collapsible: None,
+                collapsible: false,
             }
         );
         assert_eq!(
             from_config_string(" ").unwrap(),
-            InstanceConfig {
+            AdmonitionInfoRaw {
                 directive: "".to_owned(),
                 title: None,
                 additional_classnames: Vec::new(),
-                collapsible: None,
+                collapsible: false,
             }
         );
         assert_eq!(
-            from_config_string(
-                r#"type="note" class="additional classname" title="Никита" collapsible=true"#
-            )
-            .unwrap(),
-            InstanceConfig {
+            from_config_string(r#"type="note" class="additional classname" title="Никита""#)
+                .unwrap(),
+            AdmonitionInfoRaw {
                 directive: "note".to_owned(),
                 title: Some("Никита".to_owned()),
                 additional_classnames: vec!["additional".to_owned(), "classname".to_owned()],
-                collapsible: Some(true),
+                collapsible: false,
             }
         );
         // Specifying unknown keys is okay, as long as they're valid
         assert_eq!(
             from_config_string(r#"unkonwn="but valid toml""#).unwrap(),
-            InstanceConfig {
+            AdmonitionInfoRaw {
                 directive: "".to_owned(),
                 title: None,
                 additional_classnames: Vec::new(),
-                collapsible: None,
+                collapsible: false,
             }
         );
         // Just directive is fine
         assert_eq!(
             from_config_string(r#"info"#).unwrap(),
-            InstanceConfig {
+            AdmonitionInfoRaw {
                 directive: "info".to_owned(),
                 title: None,
                 additional_classnames: Vec::new(),
-                collapsible: None,
+                collapsible: false,
             }
         );
         // Directive plus toml config
         assert_eq!(
-            from_config_string(r#"info title="Information" collapsible=false"#).unwrap(),
-            InstanceConfig {
+            from_config_string(r#"info title="Information""#).unwrap(),
+            AdmonitionInfoRaw {
                 directive: "info".to_owned(),
                 title: Some("Information".to_owned()),
                 additional_classnames: Vec::new(),
-                collapsible: Some(false),
+                collapsible: false,
             }
         );
         // Directive after toml config is an error
@@ -168,12 +166,7 @@ mod test {
     fn test_from_config_string_invalid_toml_value() {
         assert_eq!(
             from_config_string(r#"note titlel=""#).unwrap_err(),
-            r#"TOML parsing error: TOML parse error at line 1, column 6
-  |
-1 | note 
-  |      ^
-expected `.`, `=`
-"#
+            "TOML parsing error: expected an equals, found a newline at line 1 column 6".to_owned()
         );
     }
 }

src/lib.rs

@@ -1,4 +1,4 @@
-use anyhow::{anyhow, Context, Result};
+use anyhow::{anyhow, Result};
 use mdbook::{
     book::{Book, BookItem},
     errors::Result as MdbookResult,
@@ -9,14 +9,9 @@ use pulldown_cmark::{CodeBlockKind::*, Event, Options, Parser, Tag};
 use std::{borrow::Cow, str::FromStr};
 
 mod config;
-mod parse;
-mod resolve;
 mod types;
 
-use crate::{
-    resolve::AdmonitionMeta,
-    types::{AdmonitionDefaults, Directive},
-};
+use crate::{config::AdmonitionInfo, types::Directive};
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 enum OnFailure {
@@ -71,7 +66,7 @@ impl Preprocessor for Admonish {
             }
 
             if let BookItem::Chapter(ref mut chapter) = *item {
-                res = Some(preprocess(&chapter.content, ctx, on_failure).map(|md| {
+                res = Some(preprocess(&chapter.content, on_failure).map(|md| {
                     chapter.content = md;
                 }));
             }
@@ -145,15 +140,15 @@ impl Directive {
 #[derive(Debug, PartialEq)]
 struct Admonition<'a> {
     directive: Directive,
-    title: String,
+    title: Option<String>,
     content: Cow<'a, str>,
     additional_classnames: Vec<String>,
     collapsible: bool,
 }
 
 impl<'a> Admonition<'a> {
-    pub fn new(info: AdmonitionMeta, content: &'a str) -> Self {
-        let AdmonitionMeta {
+    pub fn new(info: AdmonitionInfo, content: &'a str) -> Self {
+        let AdmonitionInfo {
             directive,
             title,
             additional_classnames,
@@ -175,19 +170,20 @@ impl<'a> Admonition<'a> {
 
         let title_block = if self.collapsible { "summary" } else { "div" };
 
-        let title_html = if !title.is_empty() {
-            Cow::Owned(format!(
-                r##"<{title_block} class="admonition-title">
+        let title_html = title
+            .as_ref()
+            .map(|title| {
+                Cow::Owned(format!(
+                    r##"<{title_block} class="admonition-title">
 
 {title}
 
 <a class="admonition-anchor-link" href="#{ANCHOR_ID_PREFIX}-{anchor_id}"></a>
 </{title_block}>
 "##
-            ))
-        } else {
-            Cow::Borrowed("")
-        };
+                ))
+            })
+            .unwrap_or(Cow::Borrowed(""));
 
         if !self.additional_classnames.is_empty() {
             let mut buffer = additional_class.into_owned();
@@ -219,6 +215,28 @@ impl<'a> Admonition<'a> {
 const ANCHOR_ID_PREFIX: &str = "admonition";
 const ANCHOR_ID_DEFAULT: &str = "default";
 
+fn extract_admonish_body(content: &str) -> &str {
+    const PRE_END: char = '\n';
+    const POST: &str = "```";
+
+    // We can't trust the info string length to find the start of the body
+    // it may change length if it contains HTML or character escapes.
+    //
+    // So we scan for the first newline and use that.
+    // If gods forbid it doesn't exist for some reason, just include the whole info string.
+    let start_index = content
+        // Start one character _after_ the newline
+        .find(PRE_END)
+        .map(|index| index + 1)
+        .unwrap_or_default();
+    let end_index = content.len() - POST.len();
+
+    let admonish_content = &content[start_index..end_index];
+    // The newline after a code block is technically optional, so we have to
+    // trim it off dynamically.
+    admonish_content.trim()
+}
+
 /// Given the content in the span of the code block, and the info string,
 /// return `Some(Admonition)` if the code block is an admonition.
 ///
@@ -230,74 +248,41 @@ const ANCHOR_ID_DEFAULT: &str = "default";
 /// If the code block is not an admonition, return `None`.
 fn parse_admonition<'a>(
     info_string: &'a str,
-    admonition_defaults: &'a AdmonitionDefaults,
     content: &'a str,
     on_failure: OnFailure,
 ) -> Option<MdbookResult<Admonition<'a>>> {
-    // We need to know fence details anyway for error messages
-    let extracted = parse::extract_admonish_body(content);
-
-    let info = AdmonitionMeta::from_info_string(info_string, admonition_defaults)?;
+    let info = AdmonitionInfo::from_info_string(info_string)?;
     let info = match info {
         Ok(info) => info,
         // FIXME return error messages to break build if configured
         // Err(message) => return Some(Err(content)),
         Err(message) => {
-            // Construct a fence capable of enclosing whatever we wrote for the
-            // actual input block
-            let fence = extracted.fence;
-            let enclosing_fence: String = std::iter::repeat(fence.character)
-                .take(fence.length + 1)
-                .collect();
             return Some(match on_failure {
                 OnFailure::Continue => Ok(Admonition {
                     directive: Directive::Bug,
-                    title: "Error rendering admonishment".to_owned(),
+                    title: Some("Error rendering admonishment".to_owned()),
                     additional_classnames: Vec::new(),
                     collapsible: false,
                     content: Cow::Owned(format!(
-                        r#"Failed with:
-
-```
-{message}
-```
+                        r#"Failed with: {message}
 
 Original markdown input:
 
-{enclosing_fence}
+``````
 {content}
-{enclosing_fence}
+``````
 "#
                     )),
                 }),
                 OnFailure::Bail => Err(anyhow!("Error processing admonition, bailing:\n{content}")),
-            });
+            })
         }
     };
-
-    Some(Ok(Admonition::new(info, extracted.body)))
+    let body = extract_admonish_body(content);
+    Some(Ok(Admonition::new(info, body)))
 }
 
-fn load_defaults(ctx: &PreprocessorContext) -> Result<AdmonitionDefaults> {
-    let table_op = ctx.config.get("preprocessor.admonish.default");
-
-    Ok(if let Some(table) = table_op {
-        table
-            .to_owned()
-            .try_into()
-            .context("preprocessor.admonish.default could not be parsed from book.toml")?
-    } else {
-        Default::default()
-    })
-}
-
-fn preprocess(
-    content: &str,
-    ctx: &PreprocessorContext,
-    on_failure: OnFailure,
-) -> MdbookResult<String> {
-    let admonition_defaults = load_defaults(ctx)?;
-
+fn preprocess(content: &str, on_failure: OnFailure) -> MdbookResult<String> {
     let mut id_counter = Default::default();
     let mut opts = Options::empty();
     opts.insert(Options::ENABLE_TABLES);
@@ -308,31 +293,19 @@ fn preprocess(
     let mut admonish_blocks = vec![];
 
     let events = Parser::new_ext(content, opts);
-
-    for (event, span) in events.into_offset_iter() {
-        if let Event::Start(Tag::CodeBlock(Fenced(info_string))) = event.clone() {
+    for (e, span) in events.into_offset_iter() {
+        if let Event::Start(Tag::CodeBlock(Fenced(info_string))) = e.clone() {
             let span_content = &content[span.start..span.end];
-
-            let admonition = match parse_admonition(
-                info_string.as_ref(),
-                &admonition_defaults,
-                span_content,
-                on_failure,
-            ) {
+            let admonition = match parse_admonition(info_string.as_ref(), span_content, on_failure)
+            {
                 Some(admonition) => admonition,
                 None => continue,
             };
-
             let admonition = admonition?;
             let anchor_id = unique_id_from_content(
-                if !admonition.title.is_empty() {
-                    &admonition.title
-                } else {
-                    ANCHOR_ID_DEFAULT
-                },
+                admonition.title.as_deref().unwrap_or(ANCHOR_ID_DEFAULT),
                 &mut id_counter,
             );
-
             admonish_blocks.push((span, admonition.html(&anchor_id)));
         }
     }
@@ -343,7 +316,6 @@ fn preprocess(
         let post_content = &content[span.end..];
         content = format!("{}\n{}{}", pre_content, block, post_content);
     }
-
     Ok(content)
 }
 
@@ -352,53 +324,8 @@ mod test {
     use super::*;
     use pretty_assertions::assert_eq;
 
-    fn create_mock_context(admonish_ops: &str) -> PreprocessorContext {
-        let input_json = format!(
-            r##"[
-                {{
-                    "root": "/path/to/book",
-                    "config": {{
-                        "book": {{
-                            "authors": ["AUTHOR"],
-                            "language": "en",
-                            "multilingual": false,
-                            "src": "src",
-                            "title": "TITLE"
-                        }},
-                        "preprocessor": {{
-                            "admonish": {admonish_ops}
-                        }}
-                    }},
-                    "renderer": "html",
-                    "mdbook_version": "0.4.21"
-                }},
-                {{
-                    "sections": [
-                        {{
-                            "Chapter": {{
-                                "name": "Chapter 1",
-                                "content": "# Chapter 1\n",
-                                "number": [1],
-                                "sub_items": [],
-                                "path": "chapter_1.md",
-                                "source_path": "chapter_1.md",
-                                "parent_names": []
-                            }}
-                        }}
-                    ],
-                    "__non_exhaustive": null
-                }}
-            ]"##
-        );
-        let input_json = input_json.as_bytes();
-
-        let (ctx, _) = mdbook::preprocess::CmdPreprocessor::parse_input(input_json).unwrap();
-        ctx
-    }
-
     fn prep(content: &str) -> String {
-        let ctx = create_mock_context("{}");
-        preprocess(content, &ctx, OnFailure::Continue).unwrap()
+        preprocess(content, OnFailure::Continue).unwrap()
     }
 
     #[test]
@@ -423,40 +350,6 @@ Note
 
 A simple admonition.
 
-</div>
-</div>
-Text
-"##;
-
-        assert_eq!(expected, prep(content));
-    }
-
-    #[test]
-    fn adds_admonish_longer_code_fence() {
-        let content = r#"# Chapter
-````admonish
-```json
-{}
-```
-````
-Text
-"#;
-
-        let expected = r##"# Chapter
-
-<div id="admonition-note" class="admonition note">
-<div class="admonition-title">
-
-Note
-
-<a class="admonition-anchor-link" href="#admonition-note"></a>
-</div>
-<div>
-
-```json
-{}
-```
-
 </div>
 </div>
 Text
@@ -487,36 +380,6 @@ Warning
 
 A simple admonition.
 
-</div>
-</div>
-Text
-"##;
-
-        assert_eq!(expected, prep(content));
-    }
-
-    #[test]
-    fn adds_admonish_directive_alternate() {
-        let content = r#"# Chapter
-```admonish caution
-A warning with alternate title.
-```
-Text
-"#;
-
-        let expected = r##"# Chapter
-
-<div id="admonition-caution" class="admonition warning">
-<div class="admonition-title">
-
-Caution
-
-<a class="admonition-anchor-link" href="#admonition-caution"></a>
-</div>
-<div>
-
-A warning with alternate title.
-
 </div>
 </div>
 Text
@@ -850,24 +713,15 @@ Error rendering admonishment
 </div>
 <div>
 
-Failed with:
-
-```
-TOML parsing error: TOML parse error at line 1, column 8
-  |
-1 | title="
-  |        ^
-invalid basic string
-
-```
+Failed with: TOML parsing error: unterminated string at line 1 column 7
 
 Original markdown input:
 
-````
+``````
 ```admonish title="
 Bonus content!
 ```
-````
+``````
 
 
 </div>
@@ -884,9 +738,9 @@ Bonus content!
 Bonus content!
 ```
 "#;
-        let ctx = create_mock_context(r#"{}"#);
+
         assert_eq!(
-            preprocess(content, &ctx, OnFailure::Bail)
+            preprocess(content, OnFailure::Bail)
                 .unwrap_err()
                 .to_string(),
             r#"Error processing admonition, bailing:
@@ -920,88 +774,6 @@ Hidden
 
 </div>
 </details>
-"##;
-
-        assert_eq!(expected, prep(content));
-    }
-
-    #[test]
-    fn default_toml_title() {
-        let content = r#"# Chapter
-```admonish
-A simple admonition.
-```
-Text
-"#;
-
-        let expected = r##"# Chapter
-
-<div id="admonition-admonish" class="admonition note">
-<div class="admonition-title">
-
-Admonish
-
-<a class="admonition-anchor-link" href="#admonition-admonish"></a>
-</div>
-<div>
-
-A simple admonition.
-
-</div>
-</div>
-Text
-"##;
-
-        let ctx = create_mock_context(r#"{"default": {"title": "Admonish"}}"#);
-        let preprocess_result = preprocess(content, &ctx, OnFailure::Continue).unwrap();
-        assert_eq!(expected, preprocess_result);
-    }
-
-    #[test]
-    fn empty_explicit_title_with_default() {
-        let content = r#"# Chapter
-```admonish title=""
-A simple admonition.
-```
-Text
-"#;
-
-        let expected = r##"# Chapter
-
-<div id="admonition-default" class="admonition note">
-<div>
-
-A simple admonition.
-
-</div>
-</div>
-Text
-"##;
-
-        let ctx = create_mock_context(r#"{"default": {"title": "Admonish"}}"#);
-        let preprocess_result = preprocess(content, &ctx, OnFailure::Continue).unwrap();
-        assert_eq!(expected, preprocess_result);
-    }
-
-    #[test]
-    fn empty_explicit_title() {
-        let content = r#"# Chapter
-```admonish title=""
-A simple admonition.
-```
-Text
-"#;
-
-        let expected = r##"# Chapter
-
-<div id="admonition-default" class="admonition note">
-<div>
-
-A simple admonition.
-
-</div>
-</div>
-Text
 "##;
 
         assert_eq!(expected, prep(content));

src/parse.rs

@@ -1,148 +0,0 @@
-/// We can't trust the info string length to find the start of the body
-/// it may change length if it contains HTML or character escapes.
-///
-/// So we scan for the first newline and use that.
-/// If gods forbid it doesn't exist for some reason, just include the whole info string.
-fn extract_admonish_body_start_index(content: &str) -> usize {
-    let index = content
-        .find('\n')
-        // Start one character _after_ the newline
-        .map(|index| index + 1);
-
-    // If we can't get a valid index, include all content
-    match index {
-        // Couldn't find a newline
-        None => 0,
-        Some(index) => {
-            // Index out of bound of content
-            if index > (content.len() - 1) {
-                0
-            } else {
-                index
-            }
-        }
-    }
-}
-
-fn extract_admonish_body_end_index(content: &str) -> (usize, Fence) {
-    let fence_character = content.chars().rev().next().unwrap_or('`');
-    let number_fence_characters = content
-        .chars()
-        .rev()
-        .position(|c| c != fence_character)
-        .unwrap_or_default();
-    let fence = Fence::new(fence_character, number_fence_characters);
-
-    let index = content.len() - fence.length;
-    (index, fence)
-}
-
-#[derive(Debug, PartialEq)]
-pub(crate) struct Fence {
-    pub(crate) character: char,
-    pub(crate) length: usize,
-}
-
-impl Fence {
-    pub fn new(character: char, length: usize) -> Self {
-        Self { character, length }
-    }
-}
-
-#[derive(Debug, PartialEq)]
-pub(crate) struct Extracted<'a> {
-    pub(crate) body: &'a str,
-    pub(crate) fence: Fence,
-}
-
-/// Given the whole text content of the code fence, extract the body.
-///
-/// This really feels like we should get the markdown parser to do it for us,
-/// but it's not really clear a good way of doing that.
-///
-/// ref: https://spec.commonmark.org/0.30/#fenced-code-blocks
-pub(crate) fn extract_admonish_body(content: &str) -> Extracted<'_> {
-    let start_index = extract_admonish_body_start_index(content);
-    let (end_index, fence) = extract_admonish_body_end_index(content);
-
-    let admonish_content = &content[start_index..end_index];
-    // The newline after a code block is technically optional, so we have to
-    // trim it off dynamically.
-    let body = admonish_content.trim_end();
-    Extracted { body, fence }
-}
-
-#[cfg(test)]
-mod test {
-    use super::*;
-    use pretty_assertions::assert_eq;
-
-    #[test]
-    fn test_extract_start() {
-        for (text, expected) in [
-            ("```sane example\ncontent```", 16),
-            ("~~~~~\nlonger fence", 6),
-            // empty
-            ("```\n```", 4),
-            // bounds check, should not index outside of content
-            ("```\n", 0),
-        ] {
-            let actual = extract_admonish_body_start_index(text);
-            assert_eq!(actual, expected);
-        }
-    }
-
-    #[test]
-    fn test_extract_end() {
-        for (text, expected) in [
-            ("\n```", (1, Fence::new('`', 3))),
-            // different lengths
-            ("\n``````", (1, Fence::new('`', 6))),
-            ("\n~~~~", (1, Fence::new('~', 4))),
-            // whitespace before fence end
-            ("\n   ```", (4, Fence::new('`', 3))),
-            ("content\n```", (8, Fence::new('`', 3))),
-        ] {
-            let actual = extract_admonish_body_end_index(text);
-            assert_eq!(actual, expected);
-        }
-    }
-
-    #[test]
-    fn test_extract() {
-        fn content_fence(body: &'static str, character: char, length: usize) -> Extracted<'static> {
-            Extracted {
-                body,
-                fence: Fence::new(character, length),
-            }
-        }
-        for (text, expected) in [
-            // empty
-            ("```\n```", content_fence("", '`', 3)),
-            // standard
-            (
-                "```admonish\ncontent\n```",
-                content_fence("content", '`', 3),
-            ),
-            // whitespace
-            (
-                "```admonish  \n  content  \n  ```",
-                content_fence("  content", '`', 3),
-            ),
-            // longer
-            (
-                "``````admonish\ncontent\n``````",
-                content_fence("content", '`', 6),
-            ),
-            // unequal
-            (
-                "~~~admonish\ncontent\n~~~~~",
-                // longer (end) fence returned
-                content_fence("content", '~', 5),
-            ),
-        ] {
-            let actual = extract_admonish_body(text);
-            assert_eq!(actual, expected);
-        }
-    }
-}

src/resolve.rs

@@ -1,92 +0,0 @@
-use crate::config::InstanceConfig;
-use crate::types::{AdmonitionDefaults, Directive};
-use std::str::FromStr;
-
-/// All information required to render an admonition.
-///
-/// i.e. all configured options have been resolved at this point.
-#[derive(Debug, PartialEq)]
-pub(crate) struct AdmonitionMeta {
-    pub directive: Directive,
-    pub title: String,
-    pub additional_classnames: Vec<String>,
-    pub collapsible: bool,
-}
-
-impl AdmonitionMeta {
-    pub fn from_info_string(
-        info_string: &str,
-        defaults: &AdmonitionDefaults,
-    ) -> Option<Result<Self, String>> {
-        InstanceConfig::from_info_string(info_string)
-            .map(|raw| raw.map(|raw| Self::resolve(raw, defaults)))
-    }
-
-    /// Combine the per-admonition configuration with global defaults (and
-    /// other logic) to resolve the values needed for rendering.
-    fn resolve(raw: InstanceConfig, defaults: &AdmonitionDefaults) -> Self {
-        let InstanceConfig {
-            directive: raw_directive,
-            title,
-            additional_classnames,
-            collapsible,
-        } = raw;
-
-        // Use values from block, else load default value
-        let title = title.or_else(|| defaults.title.clone());
-        let collapsible = collapsible.or(defaults.collapsible).unwrap_or_default();
-
-        // Load the directive (and title, if one still not given)
-        let (directive, title) = match (Directive::from_str(&raw_directive), title) {
-            (Ok(directive), None) => (directive, ucfirst(&raw_directive)),
-            (Err(_), None) => (Directive::Note, "Note".to_owned()),
-            (Ok(directive), Some(title)) => (directive, title),
-            (Err(_), Some(title)) => (Directive::Note, title),
-        };
-
-        Self {
-            directive,
-            title,
-            additional_classnames,
-            collapsible,
-        }
-    }
-}
-
-/// Make the first letter of `input` upppercase.
-///
-/// source: https://stackoverflow.com/a/38406885
-fn ucfirst(input: &str) -> String {
-    let mut chars = input.chars();
-    match chars.next() {
-        None => String::new(),
-        Some(f) => f.to_uppercase().collect::<String>() + chars.as_str(),
-    }
-}
-
-#[cfg(test)]
-mod test {
-    use super::*;
-    use pretty_assertions::assert_eq;
-
-    #[test]
-    fn test_admonition_info_from_raw() {
-        assert_eq!(
-            AdmonitionMeta::resolve(
-                InstanceConfig {
-                    directive: " ".to_owned(),
-                    title: None,
-                    additional_classnames: Vec::new(),
-                    collapsible: None,
-                },
-                &Default::default()
-            ),
-            AdmonitionMeta {
-                directive: Directive::Note,
-                title: "Note".to_owned(),
-                additional_classnames: Vec::new(),
-                collapsible: false,
-            }
-        );
-    }
-}

src/types.rs

@@ -1,16 +1,5 @@
-use serde::{Deserialize, Serialize};
 use std::str::FromStr;
 
-/// Book wide defaults that may be provided by the user.
-#[derive(Deserialize, Serialize, Debug, Default)]
-pub(crate) struct AdmonitionDefaults {
-    #[serde(default)]
-    pub(crate) title: Option<String>,
-
-    #[serde(default)]
-    pub(crate) collapsible: Option<bool>,
-}
-
 #[derive(Debug, PartialEq)]
 pub(crate) enum Directive {
     Note,