From 1a1668a68ab94b261293062f6f3abe28ba387919 Mon Sep 17 00:00:00 2001 From: d1t2 Date: Sun, 21 Jul 2024 12:16:01 +0800 Subject: [PATCH] fix: image not found in README.md while mdbook build Since commit 1ca2a860, all assets embed to top directory in book and could not be found on reading. Another issue is not handle README.md path correctly while applying mdbook link preprocessor. This commit fixes above issues and update an integration test to ensure assets is embedded to expected path. svg image is from https://commons.wikimedia.org/wiki/File:Epub_logo.svg --- src/errors.rs | 5 ++++- src/resources/asset.rs | 17 ++++++++--------- src/resources/resource.rs | 23 +++++++++++++++++------ tests/dummy/src/02_advanced/Epub_logo.svg | 10 ++++++++++ tests/dummy/src/02_advanced/README.md | 5 +++++ tests/dummy/src/SUMMARY.md | 4 ++++ tests/integration_tests.rs | 11 ++++++++--- 7 files changed, 56 insertions(+), 19 deletions(-) create mode 100644 tests/dummy/src/02_advanced/Epub_logo.svg create mode 100644 tests/dummy/src/02_advanced/README.md diff --git a/src/errors.rs b/src/errors.rs index 97972ffab..f741cd862 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,5 +1,5 @@ -use thiserror::Error; use std::path::PathBuf; +use thiserror::Error; #[derive(Error, Debug)] pub enum Error { @@ -42,6 +42,9 @@ pub enum Error { #[error(transparent)] Io(#[from] std::io::Error), + #[error(transparent)] + AssetOutsideSrcDir(#[from] std::path::StripPrefixError), + #[error(transparent)] Book(#[from] mdbook::errors::Error), #[error(transparent)] diff --git a/src/resources/asset.rs b/src/resources/asset.rs index 57e9c5879..7f981860d 100644 --- a/src/resources/asset.rs +++ b/src/resources/asset.rs @@ -82,14 +82,7 @@ impl Asset { return Err(Error::AssetFile(absolute_location)); } // Use filename as embedded file path with content from absolute_location. - let binding = utils::normalize_path(Path::new(link)); - debug!("Extracting file name from = {:?}, binding = '{binding:?}'", &full_filename.display()); - let filename = if cfg!(target_os = "windows") { - binding.as_os_str().to_os_string() - .into_string().expect("Error getting filename for Local Asset").replace('\\', "/") - } else { - String::from(binding.as_path().to_str().unwrap()) - }; + let filename = full_filename.strip_prefix(src_dir)?; let asset = Asset::new( filename, @@ -112,7 +105,13 @@ impl Asset { pub(crate) fn compute_asset_path_by_src_and_link(link: &str, chapter_dir: &PathBuf) -> PathBuf { let mut reassigned_asset_root: PathBuf = PathBuf::from(chapter_dir); let link_string = String::from(link); - // if chapter is a MD file, remove if from path + // mdbook built-in link preprocessor have `README.md` renamed and `index.md` is not exist + // strip the converted filename in the path + if chapter_dir.ends_with("index.md") && !chapter_dir.exists() { + debug!("It seems a `README.md` chapter, adjust the chapter root."); + reassigned_asset_root.pop(); + } + // if chapter is a MD file or not exist, remove if from path if chapter_dir.is_file() { reassigned_asset_root.pop(); } diff --git a/src/resources/resource.rs b/src/resources/resource.rs index 9735fcaaf..8af932f4f 100644 --- a/src/resources/resource.rs +++ b/src/resources/resource.rs @@ -48,9 +48,15 @@ pub(crate) fn find(ctx: &RenderContext) -> Result, Error> continue; } for link in find_assets_in_markdown(&ch.content)? { - let asset = match Url::parse(&link) { - Ok(url) => Asset::from_url(url, &ctx.destination), - Err(_) => Asset::from_local(&link, &src_dir, ch.path.as_ref().unwrap()), + let asset = if let Ok(url) = Url::parse(&link) { + Asset::from_url(url, &ctx.destination) + } else { + let result = Asset::from_local(&link, &src_dir, ch.path.as_ref().unwrap()); + if let Err(Error::AssetOutsideSrcDir(_)) = result { + warn!("Asset '{link}' is outside source dir '{src_dir:?}' and ignored"); + continue; + }; + result }?; // that is CORRECT generation way @@ -137,7 +143,12 @@ fn find_assets_in_markdown(chapter_src_content: &str) -> Result, Err // that will process chapter content and find assets for event in pull_down_parser { match event { - Event::Start(Tag::Image{link_type: _, dest_url, title: _, id: _}) => { + Event::Start(Tag::Image { + link_type: _, + dest_url, + title: _, + id: _, + }) => { found_asset.push(dest_url.to_string()); } Event::Html(html) | Event::InlineHtml(html) => { @@ -166,10 +177,10 @@ fn find_assets_in_markdown(chapter_src_content: &str) -> Result, Err #[cfg(test)] mod tests { - use std::path::{Path, PathBuf}; + use super::*; use serde_json::{json, Value}; + use std::path::{Path, PathBuf}; use tempfile::TempDir; - use super::*; #[test] fn test_find_images() { diff --git a/tests/dummy/src/02_advanced/Epub_logo.svg b/tests/dummy/src/02_advanced/Epub_logo.svg new file mode 100644 index 000000000..15623283a --- /dev/null +++ b/tests/dummy/src/02_advanced/Epub_logo.svg @@ -0,0 +1,10 @@ + + + + + + diff --git a/tests/dummy/src/02_advanced/README.md b/tests/dummy/src/02_advanced/README.md new file mode 100644 index 000000000..314d60bcb --- /dev/null +++ b/tests/dummy/src/02_advanced/README.md @@ -0,0 +1,5 @@ +# README.md tests + +## image link + +![Awesome image in the same folder](Epub_logo_color.svg "Awesome") diff --git a/tests/dummy/src/SUMMARY.md b/tests/dummy/src/SUMMARY.md index dd840d0f7..2d62376e5 100644 --- a/tests/dummy/src/SUMMARY.md +++ b/tests/dummy/src/SUMMARY.md @@ -2,3 +2,7 @@ - [Chapter 1](./chapter_1.md) - [Getting started](01_getting_started/02_article.md) + +# Advanced + +- [README.md tests](./02_advanced/README.md) diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index dd1283c48..6e8bbe228 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -130,12 +130,17 @@ fn look_for_chapter_1_heading() { fn rendered_document_contains_all_chapter_files_and_assets() { init_logging(); debug!("rendered_document_contains_all_chapter_files_and_assets..."); - let chapters = vec!["chapter_1.html", "rust-logo.png"]; + let chapters = vec![ + "chapter_1.html", + "rust-logo.png", + "02_advanced/README.html", + "02_advanced/Epub_logo_color.svg", + ]; let mut doc = generate_epub().unwrap(); debug!("Number of internal epub resources = {:?}", doc.0.resources); // number of internal epub resources for dummy test book - assert_eq!(10, doc.0.resources.len()); - assert_eq!(2, doc.0.spine.len()); + assert_eq!(12, doc.0.resources.len()); + assert_eq!(3, doc.0.spine.len()); assert_eq!(doc.0.mdata("title").unwrap(), "DummyBook"); assert_eq!(doc.0.mdata("language").unwrap(), "en"); debug!(