From 797112ef360fcd25a6d294fe5100b911fbb00e95 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sat, 20 Sep 2025 17:05:33 -0700 Subject: [PATCH] Clean up some fs-related utilities This does a little cleanup around the usage of filesystem functions: - Add `mdbook_core::utils::fs::read_to_string` as a wrapper around `std::fs::read_to_string` to provide better error messages. Use this wherever a file is read. - Add `mdbook_core::utils::fs::create_dir_all` as a wrapper around `std::fs::create_dir_all` to provide better error messages. Use this wherever a file is read. - Replace `mdbook_core::utils::fs::write_file` with `write` to mirror the `std::fs::write` API. - Remove `mdbook_core::utils::fs::create_file`. It was generally not used anymore. - Scrub the usage of `std::fs` to use the new wrappers. This doesn't remove it 100%, but it is now significantly reduced. --- crates/mdbook-core/src/config.rs | 14 +-- crates/mdbook-core/src/utils/fs.rs | 106 ++++++++---------- .../src/builtin_preprocessors/links.rs | 2 +- .../builtin_renderers/markdown_renderer.rs | 14 +-- .../src/builtin_renderers/mod.rs | 2 +- crates/mdbook-driver/src/init.rs | 97 +++++----------- crates/mdbook-driver/src/load.rs | 47 ++------ crates/mdbook-driver/src/mdbook.rs | 7 +- .../src/html_handlebars/hbs_renderer.rs | 64 +++++------ .../src/html_handlebars/static_files.rs | 36 +++--- crates/mdbook-html/src/theme/mod.rs | 8 +- tests/testsuite/book_test.rs | 14 +-- tests/testsuite/build.rs | 2 +- 13 files changed, 159 insertions(+), 254 deletions(-) diff --git a/crates/mdbook-core/src/config.rs b/crates/mdbook-core/src/config.rs index 6ca953ebfa..6f2a8a7a03 100644 --- a/crates/mdbook-core/src/config.rs +++ b/crates/mdbook-core/src/config.rs @@ -43,14 +43,11 @@ //! # run().unwrap() //! ``` -use crate::utils::TomlExt; -use crate::utils::log_backtrace; +use crate::utils::{TomlExt, fs, log_backtrace}; use anyhow::{Context, Error, Result, bail}; use serde::{Deserialize, Serialize}; use std::collections::{BTreeMap, HashMap}; use std::env; -use std::fs::File; -use std::io::Read; use std::path::{Path, PathBuf}; use std::str::FromStr; use toml::Value; @@ -113,13 +110,8 @@ impl Default for Config { impl Config { /// Load the configuration file from disk. pub fn from_disk>(config_file: P) -> Result { - let mut buffer = String::new(); - File::open(config_file) - .with_context(|| "Unable to open the configuration file")? - .read_to_string(&mut buffer) - .with_context(|| "Couldn't read the file")?; - - Config::from_str(&buffer) + let cfg = fs::read_to_string(config_file)?; + Config::from_str(&cfg) } /// Updates the `Config` from the available environment variables. diff --git a/crates/mdbook-core/src/utils/fs.rs b/crates/mdbook-core/src/utils/fs.rs index 6ad2a9dd20..2e61d87d93 100644 --- a/crates/mdbook-core/src/utils/fs.rs +++ b/crates/mdbook-core/src/utils/fs.rs @@ -1,16 +1,38 @@ //! Filesystem utilities and helpers. use anyhow::{Context, Result}; -use std::fs::{self, File}; -use std::io::Write; +use std::fs; use std::path::{Component, Path, PathBuf}; -use tracing::{debug, trace}; +use tracing::debug; -/// Write the given data to a file, creating it first if necessary -pub fn write_file>(build_dir: &Path, filename: P, content: &[u8]) -> Result<()> { - let path = build_dir.join(filename); +/// Reads a file into a string. +/// +/// Equivalent to [`std::fs::read_to_string`] with better error messages. +pub fn read_to_string>(path: P) -> Result { + let path = path.as_ref(); + fs::read_to_string(path).with_context(|| format!("failed to read `{}`", path.display())) +} - create_file(&path)?.write_all(content).map_err(Into::into) +/// Writes a file to disk. +/// +/// Equivalent to [`std::fs::write`] with better error messages. This will +/// also create the parent directory if it doesn't exist. +pub fn write, C: AsRef<[u8]>>(path: P, contents: C) -> Result<()> { + let path = path.as_ref(); + debug!("Writing `{}`", path.display()); + if let Some(parent) = path.parent() { + create_dir_all(parent)?; + } + fs::write(path, contents.as_ref()) + .with_context(|| format!("failed to write `{}`", path.display())) +} + +/// Equivalent to [`std::fs::create_dir_all`] with better error messages. +pub fn create_dir_all(p: impl AsRef) -> Result<()> { + let p = p.as_ref(); + fs::create_dir_all(p) + .with_context(|| format!("failed to create directory `{}`", p.display()))?; + Ok(()) } /// Takes a path and returns a path containing just enough `../` to point to @@ -48,30 +70,19 @@ pub fn path_to_root>(path: P) -> String { }) } -/// This function creates a file and returns it. But before creating the file -/// it checks every directory in the path to see if it exists, -/// and if it does not it will be created. -pub fn create_file(path: &Path) -> Result { - debug!("Creating {}", path.display()); - - // Construct path - if let Some(p) = path.parent() { - trace!("Parent directory is: {:?}", p); - - fs::create_dir_all(p)?; - } - - File::create(path).map_err(Into::into) -} - -/// Removes all the content of a directory but not the directory itself +/// Removes all the content of a directory but not the directory itself. pub fn remove_dir_content(dir: &Path) -> Result<()> { - for item in fs::read_dir(dir)?.flatten() { + for item in fs::read_dir(dir) + .with_context(|| format!("failed to read directory `{}`", dir.display()))? + .flatten() + { let item = item.path(); if item.is_dir() { - fs::remove_dir_all(item)?; + fs::remove_dir_all(&item) + .with_context(|| format!("failed to remove `{}`", item.display()))?; } else { - fs::remove_file(item)?; + fs::remove_file(&item) + .with_context(|| format!("failed to remove `{}`", item.display()))?; } } Ok(()) @@ -162,7 +173,7 @@ fn copy, Q: AsRef>(from: P, to: Q) -> Result<()> { use std::fs::OpenOptions; use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; - let mut reader = File::open(from)?; + let mut reader = std::fs::File::open(from)?; let metadata = reader.metadata()?; if !metadata.is_file() { anyhow::bail!( @@ -198,8 +209,9 @@ fn copy, Q: AsRef>(from: P, to: Q) -> Result<()> { #[cfg(test)] mod tests { - use super::copy_files_except_ext; - use std::{fs, io::Result, path::Path}; + use super::*; + use std::io::Result; + use std::path::Path; #[cfg(target_os = "windows")] fn symlink, Q: AsRef>(src: P, dst: Q) -> Result<()> { @@ -219,38 +231,18 @@ mod tests { }; // Create a couple of files - if let Err(err) = fs::File::create(tmp.path().join("file.txt")) { - panic!("Could not create file.txt: {err}"); - } - if let Err(err) = fs::File::create(tmp.path().join("file.md")) { - panic!("Could not create file.md: {err}"); - } - if let Err(err) = fs::File::create(tmp.path().join("file.png")) { - panic!("Could not create file.png: {err}"); - } - if let Err(err) = fs::create_dir(tmp.path().join("sub_dir")) { - panic!("Could not create sub_dir: {err}"); - } - if let Err(err) = fs::File::create(tmp.path().join("sub_dir/file.png")) { - panic!("Could not create sub_dir/file.png: {err}"); - } - if let Err(err) = fs::create_dir(tmp.path().join("sub_dir_exists")) { - panic!("Could not create sub_dir_exists: {err}"); - } - if let Err(err) = fs::File::create(tmp.path().join("sub_dir_exists/file.txt")) { - panic!("Could not create sub_dir_exists/file.txt: {err}"); - } + write(tmp.path().join("file.txt"), "").unwrap(); + write(tmp.path().join("file.md"), "").unwrap(); + write(tmp.path().join("file.png"), "").unwrap(); + write(tmp.path().join("sub_dir/file.png"), "").unwrap(); + write(tmp.path().join("sub_dir_exists/file.txt"), "").unwrap(); if let Err(err) = symlink(tmp.path().join("file.png"), tmp.path().join("symlink.png")) { panic!("Could not symlink file.png: {err}"); } // Create output dir - if let Err(err) = fs::create_dir(tmp.path().join("output")) { - panic!("Could not create output: {err}"); - } - if let Err(err) = fs::create_dir(tmp.path().join("output/sub_dir_exists")) { - panic!("Could not create output/sub_dir_exists: {err}"); - } + create_dir_all(tmp.path().join("output")).unwrap(); + create_dir_all(tmp.path().join("output/sub_dir_exists")).unwrap(); if let Err(e) = copy_files_except_ext(tmp.path(), &tmp.path().join("output"), true, None, &["md"]) diff --git a/crates/mdbook-driver/src/builtin_preprocessors/links.rs b/crates/mdbook-driver/src/builtin_preprocessors/links.rs index 787b7745da..e369086fb4 100644 --- a/crates/mdbook-driver/src/builtin_preprocessors/links.rs +++ b/crates/mdbook-driver/src/builtin_preprocessors/links.rs @@ -5,9 +5,9 @@ use self::take_lines::{ use anyhow::{Context, Result}; use mdbook_core::book::{Book, BookItem}; use mdbook_core::static_regex; +use mdbook_core::utils::fs; use mdbook_preprocessor::{Preprocessor, PreprocessorContext}; use regex::{CaptureMatches, Captures}; -use std::fs; use std::ops::{Bound, Range, RangeBounds, RangeFrom, RangeFull, RangeTo}; use std::path::{Path, PathBuf}; use tracing::{error, warn}; diff --git a/crates/mdbook-driver/src/builtin_renderers/markdown_renderer.rs b/crates/mdbook-driver/src/builtin_renderers/markdown_renderer.rs index 4c9931ab53..8b6680e8c5 100644 --- a/crates/mdbook-driver/src/builtin_renderers/markdown_renderer.rs +++ b/crates/mdbook-driver/src/builtin_renderers/markdown_renderer.rs @@ -1,7 +1,6 @@ use anyhow::{Context, Result}; -use mdbook_core::utils; +use mdbook_core::utils::fs; use mdbook_renderer::{RenderContext, Renderer}; -use std::fs; use tracing::trace; /// A renderer to output the Markdown after the preprocessors have run. Mostly useful @@ -27,17 +26,16 @@ impl Renderer for MarkdownRenderer { let book = &ctx.book; if destination.exists() { - utils::fs::remove_dir_content(destination) + fs::remove_dir_content(destination) .with_context(|| "Unable to remove stale Markdown output")?; } trace!("markdown render"); for ch in book.chapters() { - utils::fs::write_file( - &ctx.destination, - ch.path.as_ref().expect("Checked path exists before"), - ch.content.as_bytes(), - )?; + let path = ctx + .destination + .join(ch.path.as_ref().expect("Checked path exists before")); + fs::write(path, &ch.content)?; } fs::create_dir_all(destination) diff --git a/crates/mdbook-driver/src/builtin_renderers/mod.rs b/crates/mdbook-driver/src/builtin_renderers/mod.rs index 65cff5c7e9..3490647351 100644 --- a/crates/mdbook-driver/src/builtin_renderers/mod.rs +++ b/crates/mdbook-driver/src/builtin_renderers/mod.rs @@ -3,8 +3,8 @@ //! The HTML renderer can be found in the [`mdbook_html`] crate. use anyhow::{Context, Result, bail}; +use mdbook_core::utils::fs; use mdbook_renderer::{RenderContext, Renderer}; -use std::fs; use std::process::Stdio; use tracing::{error, info, trace, warn}; diff --git a/crates/mdbook-driver/src/init.rs b/crates/mdbook-driver/src/init.rs index 78a2de6364..83785cef0a 100644 --- a/crates/mdbook-driver/src/init.rs +++ b/crates/mdbook-driver/src/init.rs @@ -1,14 +1,11 @@ //! Support for initializing a new book. -use std::fs::{self, File}; -use std::io::Write; -use std::path::PathBuf; - use super::MDBook; use anyhow::{Context, Result}; use mdbook_core::config::Config; -use mdbook_core::utils::fs::write_file; +use mdbook_core::utils::fs; use mdbook_html::theme; +use std::path::PathBuf; use tracing::{debug, error, info, trace}; /// A helper for setting up a new book and its directory structure. @@ -104,8 +101,7 @@ impl BookBuilder { let cfg = toml::to_string(&self.config).with_context(|| "Unable to serialize the config")?; - std::fs::write(&book_toml, cfg) - .with_context(|| format!("Unable to write config to {book_toml:?}"))?; + fs::write(&book_toml, cfg)?; Ok(()) } @@ -115,61 +111,32 @@ impl BookBuilder { let html_config = self.config.html_config().unwrap_or_default(); let themedir = html_config.theme_dir(&self.root); - if !themedir.exists() { - debug!( - "{} does not exist, creating the directory", - themedir.display() - ); - fs::create_dir(&themedir)?; - } - - let mut index = File::create(themedir.join("index.hbs"))?; - index.write_all(theme::INDEX)?; + fs::write(themedir.join("book.js"), theme::JS)?; + fs::write(themedir.join("favicon.png"), theme::FAVICON_PNG)?; + fs::write(themedir.join("favicon.svg"), theme::FAVICON_SVG)?; + fs::write(themedir.join("highlight.css"), theme::HIGHLIGHT_CSS)?; + fs::write(themedir.join("highlight.js"), theme::HIGHLIGHT_JS)?; + fs::write(themedir.join("index.hbs"), theme::INDEX)?; let cssdir = themedir.join("css"); - if !cssdir.exists() { - fs::create_dir(&cssdir)?; - } - - let mut general_css = File::create(cssdir.join("general.css"))?; - general_css.write_all(theme::GENERAL_CSS)?; - - let mut chrome_css = File::create(cssdir.join("chrome.css"))?; - chrome_css.write_all(theme::CHROME_CSS)?; + fs::write(cssdir.join("general.css"), theme::GENERAL_CSS)?; + fs::write(cssdir.join("chrome.css"), theme::CHROME_CSS)?; + fs::write(cssdir.join("variables.css"), theme::VARIABLES_CSS)?; if html_config.print.enable { - let mut print_css = File::create(cssdir.join("print.css"))?; - print_css.write_all(theme::PRINT_CSS)?; + fs::write(cssdir.join("print.css"), theme::PRINT_CSS)?; } - let mut variables_css = File::create(cssdir.join("variables.css"))?; - variables_css.write_all(theme::VARIABLES_CSS)?; - - let mut favicon = File::create(themedir.join("favicon.png"))?; - favicon.write_all(theme::FAVICON_PNG)?; - - let mut favicon = File::create(themedir.join("favicon.svg"))?; - favicon.write_all(theme::FAVICON_SVG)?; - - let mut js = File::create(themedir.join("book.js"))?; - js.write_all(theme::JS)?; - - let mut highlight_css = File::create(themedir.join("highlight.css"))?; - highlight_css.write_all(theme::HIGHLIGHT_CSS)?; - - let mut highlight_js = File::create(themedir.join("highlight.js"))?; - highlight_js.write_all(theme::HIGHLIGHT_JS)?; - - write_file(&themedir.join("fonts"), "fonts.css", theme::fonts::CSS)?; + let fonts_dir = themedir.join("fonts"); + fs::write(fonts_dir.join("fonts.css"), theme::fonts::CSS)?; for (file_name, contents) in theme::fonts::LICENSES { - write_file(&themedir, file_name, contents)?; + fs::write(themedir.join(file_name), contents)?; } for (file_name, contents) in theme::fonts::OPEN_SANS.iter() { - write_file(&themedir, file_name, contents)?; + fs::write(themedir.join(file_name), contents)?; } - write_file( - &themedir, - theme::fonts::SOURCE_CODE_PRO.0, + fs::write( + themedir.join(theme::fonts::SOURCE_CODE_PRO.0), theme::fonts::SOURCE_CODE_PRO.1, )?; @@ -177,12 +144,10 @@ impl BookBuilder { } fn build_gitignore(&self) -> Result<()> { - debug!("Creating .gitignore"); - - let mut f = File::create(self.root.join(".gitignore"))?; - - writeln!(f, "{}", self.config.build.build_dir.display())?; - + fs::write( + self.root.join(".gitignore"), + format!("{}", self.config.build.build_dir.display()), + )?; Ok(()) } @@ -193,14 +158,14 @@ impl BookBuilder { let summary = src_dir.join("SUMMARY.md"); if !summary.exists() { trace!("No summary found creating stub summary and chapter_1.md."); - let mut f = File::create(&summary).with_context(|| "Unable to create SUMMARY.md")?; - writeln!(f, "# Summary")?; - writeln!(f)?; - writeln!(f, "- [Chapter 1](./chapter_1.md)")?; - - let chapter_1 = src_dir.join("chapter_1.md"); - let mut f = File::create(chapter_1).with_context(|| "Unable to create chapter_1.md")?; - writeln!(f, "# Chapter 1")?; + fs::write( + summary, + "# Summary\n\ + \n\ + - [Chapter 1](./chapter_1.md)\n", + )?; + + fs::write(src_dir.join("chapter_1.md"), "# Chapter 1\n")?; } else { trace!("Existing summary found, no need to create stub files."); } diff --git a/crates/mdbook-driver/src/load.rs b/crates/mdbook-driver/src/load.rs index 9a69579cea..b7f6748bb3 100644 --- a/crates/mdbook-driver/src/load.rs +++ b/crates/mdbook-driver/src/load.rs @@ -1,10 +1,8 @@ use anyhow::{Context, Result}; use mdbook_core::book::{Book, BookItem, Chapter}; use mdbook_core::config::BuildConfig; -use mdbook_core::utils::escape_html; +use mdbook_core::utils::{escape_html, fs}; use mdbook_summary::{Link, Summary, SummaryItem, parse_summary}; -use std::fs::{self, File}; -use std::io::{Read, Write}; use std::path::Path; use tracing::debug; @@ -13,11 +11,7 @@ pub(crate) fn load_book>(src_dir: P, cfg: &BuildConfig) -> Result let src_dir = src_dir.as_ref(); let summary_md = src_dir.join("SUMMARY.md"); - let mut summary_content = String::new(); - File::open(&summary_md) - .with_context(|| format!("Couldn't open SUMMARY.md in {src_dir:?} directory"))? - .read_to_string(&mut summary_content)?; - + let summary_content = fs::read_to_string(&summary_md)?; let summary = parse_summary(&summary_content) .with_context(|| format!("Summary parsing failed for file={summary_md:?}"))?; @@ -47,12 +41,8 @@ fn create_missing(src_dir: &Path, summary: &Summary) -> Result<()> { } } debug!("Creating missing file {}", filename.display()); - - let mut f = File::create(&filename).with_context(|| { - format!("Unable to create missing file: {}", filename.display()) - })?; let title = escape_html(&link.name); - writeln!(f, "# {title}")?; + fs::write(&filename, format!("# {title}\n"))?; } } @@ -116,13 +106,8 @@ fn load_chapter>( src_dir.join(link_location) }; - let mut f = File::open(&location) - .with_context(|| format!("Chapter file not found, {}", link_location.display()))?; - - let mut content = String::new(); - f.read_to_string(&mut content).with_context(|| { - format!("Unable to read \"{}\" ({})", link.name, location.display()) - })?; + let mut content = std::fs::read_to_string(&location) + .with_context(|| format!("failed to read chapter `{}`", link_location.display()))?; if content.as_bytes().starts_with(b"\xef\xbb\xbf") { content.replace_range(..3, ""); @@ -174,10 +159,7 @@ And here is some \ let temp = TempFileBuilder::new().prefix("book").tempdir().unwrap(); let chapter_path = temp.path().join("chapter_1.md"); - File::create(&chapter_path) - .unwrap() - .write_all(DUMMY_SRC.as_bytes()) - .unwrap(); + fs::write(&chapter_path, DUMMY_SRC).unwrap(); let link = Link::new("Chapter 1", chapter_path); @@ -189,11 +171,7 @@ And here is some \ let (mut root, temp_dir) = dummy_link(); let second_path = temp_dir.path().join("second.md"); - - File::create(&second_path) - .unwrap() - .write_all(b"Hello World!") - .unwrap(); + fs::write(&second_path, "Hello World!").unwrap(); let mut second = Link::new("Nested Chapter 1", &second_path); second.number = Some(SectionNumber::new([1, 2])); @@ -224,10 +202,7 @@ And here is some \ let temp_dir = TempFileBuilder::new().prefix("book").tempdir().unwrap(); let chapter_path = temp_dir.path().join("chapter_1.md"); - File::create(&chapter_path) - .unwrap() - .write_all(("\u{feff}".to_owned() + DUMMY_SRC).as_bytes()) - .unwrap(); + fs::write(&chapter_path, format!("\u{feff}{DUMMY_SRC}")).unwrap(); let link = Link::new("Chapter 1", chapter_path); @@ -307,7 +282,7 @@ And here is some \ fn cant_load_chapters_when_the_link_is_a_directory() { let (_, temp) = dummy_link(); let dir = temp.path().join("nested"); - fs::create_dir(&dir).unwrap(); + fs::create_dir_all(&dir).unwrap(); let mut summary = Summary::default(); let link = Link::new("nested", dir); @@ -326,8 +301,8 @@ And here is some \ assert!(got.is_err()); let error_message = got.err().unwrap().to_string(); let expected = format!( - r#"Couldn't open SUMMARY.md in {:?} directory"#, - temp_dir.path() + r#"failed to read `{}`"#, + temp_dir.path().join("SUMMARY.md").display() ); assert_eq!(error_message, expected); } diff --git a/crates/mdbook-driver/src/mdbook.rs b/crates/mdbook-driver/src/mdbook.rs index 27683bd737..80ae692b92 100644 --- a/crates/mdbook-driver/src/mdbook.rs +++ b/crates/mdbook-driver/src/mdbook.rs @@ -8,14 +8,14 @@ use anyhow::{Context, Error, Result, bail}; use indexmap::IndexMap; use mdbook_core::book::{Book, BookItem, BookItems}; use mdbook_core::config::{Config, RustEdition}; -use mdbook_core::utils; +use mdbook_core::utils::fs; use mdbook_html::HtmlHandlebars; use mdbook_preprocessor::{Preprocessor, PreprocessorContext}; use mdbook_renderer::{RenderContext, Renderer}; use mdbook_summary::Summary; use serde::Deserialize; use std::ffi::OsString; -use std::io::{IsTerminal, Write}; +use std::io::IsTerminal; use std::path::{Path, PathBuf}; use std::process::Command; use tempfile::Builder as TempFileBuilder; @@ -292,8 +292,7 @@ impl MDBook { // write preprocessed file to tempdir let path = temp_dir.path().join(chapter_path); - let mut tmpf = utils::fs::create_file(&path)?; - tmpf.write_all(ch.content.as_bytes())?; + fs::write(&path, &ch.content)?; let mut cmd = Command::new("rustdoc"); cmd.current_dir(temp_dir.path()) diff --git a/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs b/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs index 832fac9acc..8edac3cace 100644 --- a/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs +++ b/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs @@ -8,11 +8,10 @@ use anyhow::{Context, Result, bail}; use handlebars::Handlebars; use mdbook_core::book::{Book, BookItem, Chapter}; use mdbook_core::config::{BookConfig, Config, HtmlConfig}; -use mdbook_core::utils; +use mdbook_core::utils::fs; use mdbook_renderer::{RenderContext, Renderer}; use serde_json::json; use std::collections::{BTreeMap, HashMap}; -use std::fs::{self, File}; use std::path::{Path, PathBuf}; use tracing::error; use tracing::{debug, info, trace, warn}; @@ -84,10 +83,8 @@ impl HtmlHandlebars { ctx.data.insert("content".to_owned(), json!(content)); ctx.data.insert("chapter_title".to_owned(), json!(ch.name)); ctx.data.insert("title".to_owned(), json!(title)); - ctx.data.insert( - "path_to_root".to_owned(), - json!(utils::fs::path_to_root(path)), - ); + ctx.data + .insert("path_to_root".to_owned(), json!(fs::path_to_root(path))); if let Some(ref section) = ch.number { ctx.data .insert("section".to_owned(), json!(section.to_string())); @@ -123,8 +120,8 @@ impl HtmlHandlebars { let rendered = ctx.handlebars.render("index", &ctx.data)?; // Write to file - debug!("Creating {}", filepath.display()); - utils::fs::write_file(&ctx.destination, &filepath, rendered.as_bytes())?; + let out_path = ctx.destination.join(filepath); + fs::write(&out_path, rendered)?; if prev_ch.is_none() { ctx.data.insert("path".to_owned(), json!("index.md")); @@ -132,7 +129,7 @@ impl HtmlHandlebars { ctx.data.insert("is_index".to_owned(), json!(true)); let rendered_index = ctx.handlebars.render("index", &ctx.data)?; debug!("Creating index.html from {}", ctx_path); - utils::fs::write_file(&ctx.destination, "index.html", rendered_index.as_bytes())?; + fs::write(ctx.destination.join("index.html"), rendered_index)?; } Ok(()) @@ -146,18 +143,15 @@ impl HtmlHandlebars { handlebars: &mut Handlebars<'_>, data: &mut serde_json::Map, ) -> Result<()> { - let destination = &ctx.destination; let content_404 = if let Some(ref filename) = html_config.input_404 { let path = src_dir.join(filename); - std::fs::read_to_string(&path) - .with_context(|| format!("unable to open 404 input file {path:?}"))? + fs::read_to_string(&path).with_context(|| "failed to read the 404 input file")? } else { // 404 input not explicitly configured try the default file 404.md let default_404_location = src_dir.join("404.md"); if default_404_location.exists() { - std::fs::read_to_string(&default_404_location).with_context(|| { - format!("unable to open 404 input file {default_404_location:?}") - })? + fs::read_to_string(&default_404_location) + .with_context(|| "failed to read the 404 input file")? } else { "# Document not found (404)\n\nThis URL is invalid, sorry. Please use the \ navigation bar or search to continue." @@ -195,8 +189,8 @@ impl HtmlHandlebars { data_404.insert("title".to_owned(), json!(title)); let rendered = handlebars.render("index", &data_404)?; - let output_file = html_config.get_404_output_file(); - utils::fs::write_file(destination, output_file, rendered.as_bytes())?; + let output_file = ctx.destination.join(html_config.get_404_output_file()); + fs::write(output_file, rendered)?; debug!("Creating 404.html ✓"); Ok(()) } @@ -222,7 +216,7 @@ impl HtmlHandlebars { data.insert("content".to_owned(), json!(print_content)); data.insert( "path_to_root".to_owned(), - json!(utils::fs::path_to_root(Path::new("print.md"))), + json!(fs::path_to_root(Path::new("print.md"))), ); debug!("Render template"); @@ -285,8 +279,7 @@ impl HtmlHandlebars { fragment_map: &BTreeMap, ) -> Result<()> { if let Some(parent) = original.parent() { - std::fs::create_dir_all(parent) - .with_context(|| format!("Unable to ensure \"{}\" exists", parent.display()))?; + fs::create_dir_all(parent)? } let js_map = serde_json::to_string(fragment_map)?; @@ -295,15 +288,13 @@ impl HtmlHandlebars { "fragment_map": js_map, "url": destination, }); - let f = File::create(original)?; - handlebars - .render_to_write("redirect", &ctx, f) - .with_context(|| { - format!( - "Unable to create a redirect file at \"{}\"", - original.display() - ) - })?; + let rendered = handlebars.render("redirect", &ctx).with_context(|| { + format!( + "Unable to create a redirect file at `{}`", + original.display() + ) + })?; + fs::write(original, rendered)?; Ok(()) } @@ -323,7 +314,7 @@ impl Renderer for HtmlHandlebars { let build_dir = ctx.root.join(&ctx.config.build.build_dir); if destination.exists() { - utils::fs::remove_dir_content(destination) + fs::remove_dir_content(destination) .with_context(|| "Unable to remove stale HTML output")?; } @@ -406,20 +397,19 @@ impl Renderer for HtmlHandlebars { data.insert("is_toc_html".to_owned(), json!(true)); data.insert("path".to_owned(), json!("toc.html")); let rendered_toc = handlebars.render("toc_html", &data)?; - utils::fs::write_file(destination, "toc.html", rendered_toc.as_bytes())?; + fs::write(destination.join("toc.html"), rendered_toc)?; debug!("Creating toc.html ✓"); data.remove("path"); data.remove("is_toc_html"); } - utils::fs::write_file( - destination, - ".nojekyll", + fs::write( + destination.join(".nojekyll"), b"This file makes sure that Github Pages doesn't process mdBook's output.\n", )?; if let Some(cname) = &html_config.cname { - utils::fs::write_file(destination, "CNAME", format!("{cname}\n").as_bytes())?; + fs::write(destination.join("CNAME"), format!("{cname}\n"))?; } for (i, chapter_tree) in chapter_trees.iter().enumerate() { @@ -446,7 +436,7 @@ impl Renderer for HtmlHandlebars { let print_rendered = self.render_print_page(ctx, &handlebars, &mut data, chapter_trees)?; - utils::fs::write_file(destination, "print.html", print_rendered.as_bytes())?; + fs::write(destination.join("print.html"), print_rendered)?; debug!("Creating print.html ✓"); } @@ -454,7 +444,7 @@ impl Renderer for HtmlHandlebars { .context("Unable to emit redirects")?; // Copy all remaining files, avoid a recursive copy from/to the book build dir - utils::fs::copy_files_except_ext(&src_dir, destination, true, Some(&build_dir), &["md"])?; + fs::copy_files_except_ext(&src_dir, destination, true, Some(&build_dir), &["md"])?; info!("HTML book written to `{}`", destination.display()); diff --git a/crates/mdbook-html/src/html_handlebars/static_files.rs b/crates/mdbook-html/src/html_handlebars/static_files.rs index 26be0c26a4..e0415cb40b 100644 --- a/crates/mdbook-html/src/html_handlebars/static_files.rs +++ b/crates/mdbook-html/src/html_handlebars/static_files.rs @@ -5,10 +5,9 @@ use crate::theme::{self, Theme, playground_editor}; use anyhow::{Context, Result}; use mdbook_core::config::HtmlConfig; use mdbook_core::static_regex; -use mdbook_core::utils; +use mdbook_core::utils::fs; use std::borrow::Cow; use std::collections::HashMap; -use std::fs::{self, File}; use std::path::{Path, PathBuf}; use tracing::debug; @@ -165,8 +164,10 @@ impl StaticFiles { if let Some((name, suffix)) = parts { if name != "" && suffix != "" { let mut digest = Sha256::new(); - let mut input_file = File::open(input_location) - .with_context(|| "open static file for hashing")?; + let mut input_file = + std::fs::File::open(input_location).with_context(|| { + format!("failed to open `{filename}` for hashing") + })?; let mut buf = vec![0; 1024]; loop { let amt = input_file @@ -190,7 +191,6 @@ impl StaticFiles { } pub(super) fn write_files(self, destination: &Path) -> Result { - use mdbook_core::utils::fs::write_file; use regex::bytes::Captures; // The `{{ resource "name" }}` directive in static resources look like // handlebars syntax, even if they technically aren't. @@ -207,7 +207,7 @@ impl StaticFiles { .as_bytes(); let name = std::str::from_utf8(name).expect("resource name with invalid utf8"); let resource_filename = hash_map.get(name).map(|s| &s[..]).unwrap_or(name); - let path_to_root = utils::fs::path_to_root(filename); + let path_to_root = fs::path_to_root(filename); format!("{}{}", path_to_root, resource_filename) .as_bytes() .to_owned() @@ -222,7 +222,8 @@ impl StaticFiles { } else { Cow::Borrowed(&data[..]) }; - write_file(destination, filename, &data)?; + let path = destination.join(filename); + fs::write(path, &data)?; } StaticFile::Additional { input_location, @@ -239,11 +240,12 @@ impl StaticFiles { .with_context(|| format!("Unable to create {}", parent.display()))?; } if filename.ends_with(".css") || filename.ends_with(".js") { - let data = fs::read(input_location)?; - let data = replace_all(&self.hash_map, &data, filename); - write_file(destination, filename, &data)?; + let data = fs::read_to_string(input_location)?; + let data = replace_all(&self.hash_map, data.as_bytes(), filename); + let path = destination.join(filename); + fs::write(path, &data)?; } else { - fs::copy(input_location, &output_location).with_context(|| { + std::fs::copy(input_location, &output_location).with_context(|| { format!( "Unable to copy {} to {}", input_location.display(), @@ -264,7 +266,7 @@ mod tests { use super::*; use crate::theme::Theme; use mdbook_core::config::HtmlConfig; - use mdbook_core::utils::fs::write_file; + use mdbook_core::utils::fs; use tempfile::TempDir; #[test] @@ -295,9 +297,8 @@ mod tests { let reference_js = Path::new("static-files-test-case-reference.js"); let mut html_config = HtmlConfig::default(); html_config.additional_js.push(reference_js.to_owned()); - write_file( - temp_dir.path(), - reference_js, + fs::write( + temp_dir.path().join(reference_js), br#"{{ resource "book.js" }}"#, ) .unwrap(); @@ -305,7 +306,7 @@ mod tests { static_files.hash_files().unwrap(); static_files.write_files(temp_dir.path()).unwrap(); // custom JS winds up referencing book.js - let reference_js_content = std::fs::read_to_string( + let reference_js_content = fs::read_to_string( temp_dir .path() .join("static-files-test-case-reference-635c9cdc.js"), @@ -313,8 +314,7 @@ mod tests { .unwrap(); assert_eq!("book-e3b0c442.js", reference_js_content); // book.js winds up empty - let book_js_content = - std::fs::read_to_string(temp_dir.path().join("book-e3b0c442.js")).unwrap(); + let book_js_content = fs::read_to_string(temp_dir.path().join("book-e3b0c442.js")).unwrap(); assert_eq!("", book_js_content); } } diff --git a/crates/mdbook-html/src/theme/mod.rs b/crates/mdbook-html/src/theme/mod.rs index ab5b0fb252..f40fbb6bf1 100644 --- a/crates/mdbook-html/src/theme/mod.rs +++ b/crates/mdbook-html/src/theme/mod.rs @@ -1,8 +1,6 @@ #![allow(missing_docs)] use anyhow::Result; -use std::fs::File; -use std::io::Read; use std::path::{Path, PathBuf}; use tracing::{info, warn}; @@ -195,9 +193,7 @@ impl Default for Theme { /// its contents. fn load_file_contents>(filename: P, dest: &mut Vec) -> Result<()> { let filename = filename.as_ref(); - - let mut buffer = Vec::new(); - File::open(filename)?.read_to_end(&mut buffer)?; + let mut buffer = std::fs::read(filename)?; // We needed the buffer so we'd only overwrite the existing content if we // could successfully load the file into memory. @@ -254,7 +250,7 @@ mod tests { // "touch" all of the special files so we have empty copies for file in &files { - File::create(&temp.path().join(file)).unwrap(); + fs::File::create(&temp.path().join(file)).unwrap(); } let got = Theme::new(temp.path()); diff --git a/tests/testsuite/book_test.rs b/tests/testsuite/book_test.rs index 0dd7a94722..f29eab9083 100644 --- a/tests/testsuite/book_test.rs +++ b/tests/testsuite/book_test.rs @@ -1,6 +1,6 @@ //! Utility for building and running tests against mdbook. -use anyhow::Context; +use mdbook_core::utils::fs; use mdbook_driver::MDBook; use mdbook_driver::init::BookBuilder; use snapbox::IntoData; @@ -76,7 +76,7 @@ impl BookTest { std::fs::remove_dir_all(&tmp) .unwrap_or_else(|e| panic!("failed to remove {tmp:?}: {e:?}")); } - std::fs::create_dir_all(&tmp).unwrap_or_else(|e| panic!("failed to create {tmp:?}: {e:?}")); + fs::create_dir_all(&tmp).unwrap(); tmp } @@ -310,7 +310,7 @@ impl BookTest { /// Change a file's contents in the given path. pub fn change_file(&mut self, path: impl AsRef, body: &str) -> &mut Self { let path = self.dir.join(path); - std::fs::write(&path, body).unwrap_or_else(|e| panic!("failed to write {path:?}: {e:?}")); + fs::write(&path, body).unwrap(); self } @@ -342,9 +342,9 @@ impl BookTest { let rs = self.dir.join(path).with_extension("rs"); let parent = rs.parent().unwrap(); if !parent.exists() { - std::fs::create_dir_all(&parent).unwrap(); + fs::create_dir_all(&parent).unwrap(); } - std::fs::write(&rs, src).unwrap_or_else(|e| panic!("failed to write {rs:?}: {e:?}")); + fs::write(&rs, src).unwrap(); let status = std::process::Command::new("rustc") .arg(&rs) .current_dir(&parent) @@ -551,9 +551,7 @@ fn assert(root: &Path) -> snapbox::Assert { #[track_caller] pub fn read_to_string>(path: P) -> String { let path = path.as_ref(); - std::fs::read_to_string(path) - .with_context(|| format!("could not read file {path:?}")) - .unwrap() + fs::read_to_string(path).unwrap() } /// Returns the first path from the given glob pattern. diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index e3851c1271..889a9e65ea 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -24,7 +24,7 @@ fn basic_build() { fn failure_on_missing_file() { BookTest::from_dir("build/missing_file").run("build", |cmd| { cmd.expect_failure().expect_stderr(str![[r#" -ERROR Chapter file not found, ./chapter_1.md +ERROR failed to read chapter `./chapter_1.md` [TAB]Caused by: [NOT_FOUND] "#]]);