From fd073cf4a39887f9d6fbeb956607101eb08a4ea1 Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Sat, 8 Oct 2016 16:29:10 -0400 Subject: [PATCH 1/5] Don't construct `PathBuf` ownership if we don't need it. --- src/librustdoc/externalfiles.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index 2ecb071fcc2a4..8b71c133ed48b 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -11,7 +11,7 @@ use std::fs::File; use std::io::prelude::*; use std::io; -use std::path::{PathBuf, Path}; +use std::path::Path; use std::str; #[derive(Clone)] @@ -47,8 +47,8 @@ pub fn load_string(input: &Path) -> io::Result> { macro_rules! load_or_return { ($input: expr, $cant_read: expr, $not_utf8: expr) => { { - let input = PathBuf::from(&$input[..]); - match ::externalfiles::load_string(&input) { + let input = Path::new(&$input[..]); + match ::externalfiles::load_string(input) { Err(e) => { let _ = writeln!(&mut io::stderr(), "error reading `{}`: {}", input.display(), e); From f410da5cbed620836df663a18350211093be686d Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Sat, 8 Oct 2016 17:34:13 -0400 Subject: [PATCH 2/5] Add doc comments describing fields on `externalfiles::ExternalHtml`. --- src/librustdoc/externalfiles.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index 8b71c133ed48b..7a6b414e79edb 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -16,8 +16,14 @@ use std::str; #[derive(Clone)] pub struct ExternalHtml{ + /// Content that will be included inline in the section of a + /// rendered Markdown file or generated documentation pub in_header: String, + /// Content that will be included inline between and the content of + /// a rendered Markdown file or generated documentation pub before_content: String, + /// Content that will be included inline between the content and of + /// a rendered Markdown file or generated documentation pub after_content: String } From 7be14eea94f1b61b3d85e29bcbefac435d0b48c2 Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Sat, 8 Oct 2016 22:55:40 -0400 Subject: [PATCH 3/5] Refactor away `load_or_return` macro. --- src/librustdoc/externalfiles.rs | 49 +++++++++++++++++---------------- src/librustdoc/markdown.rs | 14 ++++++++-- 2 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index 7a6b414e79edb..79e30b0d6aad3 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -43,30 +43,29 @@ impl ExternalHtml { } } -pub fn load_string(input: &Path) -> io::Result> { - let mut f = File::open(input)?; - let mut d = Vec::new(); - f.read_to_end(&mut d)?; - Ok(str::from_utf8(&d).map(|s| s.to_string()).ok()) +pub enum LoadStringError { + ReadFail, + BadUtf8, } -macro_rules! load_or_return { - ($input: expr, $cant_read: expr, $not_utf8: expr) => { - { - let input = Path::new(&$input[..]); - match ::externalfiles::load_string(input) { - Err(e) => { - let _ = writeln!(&mut io::stderr(), - "error reading `{}`: {}", input.display(), e); - return $cant_read; - } - Ok(None) => { - let _ = writeln!(&mut io::stderr(), - "error reading `{}`: not UTF-8", input.display()); - return $not_utf8; - } - Ok(Some(s)) => s - } +pub fn load_string>(file_path: P) -> Result { + let file_path = file_path.as_ref(); + let mut contents = vec![]; + let result = File::open(file_path) + .and_then(|mut f| f.read_to_end(&mut contents)); + if let Err(e) = result { + let _ = writeln!(&mut io::stderr(), + "error reading `{}`: {}", + file_path.display(), e); + return Err(LoadStringError::ReadFail); + } + match str::from_utf8(&contents) { + Ok(s) => Ok(s.to_string()), + Err(_) => { + let _ = writeln!(&mut io::stderr(), + "error reading `{}`: not UTF-8", + file_path.display()); + Err(LoadStringError::BadUtf8) } } } @@ -74,7 +73,11 @@ macro_rules! load_or_return { pub fn load_external_files(names: &[String]) -> Option { let mut out = String::new(); for name in names { - out.push_str(&*load_or_return!(&name, None, None)); + let s = match load_string(name) { + Ok(s) => s, + Err(_) => return None, + }; + out.push_str(&s); out.push('\n'); } Some(out) diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs index 1421a3c78fc5a..f708aa5461999 100644 --- a/src/librustdoc/markdown.rs +++ b/src/librustdoc/markdown.rs @@ -19,7 +19,7 @@ use testing; use rustc::session::search_paths::SearchPaths; use rustc::session::config::Externs; -use externalfiles::ExternalHtml; +use externalfiles::{ExternalHtml, LoadStringError, load_string}; use html::render::reset_ids; use html::escape::Escape; @@ -58,7 +58,11 @@ pub fn render(input: &str, mut output: PathBuf, matches: &getopts::Matches, css.push_str(&s) } - let input_str = load_or_return!(input, 1, 2); + let input_str = match load_string(input) { + Ok(s) => s, + Err(LoadStringError::ReadFail) => return 1, + Err(LoadStringError::BadUtf8) => return 2, + }; let playground = matches.opt_str("markdown-playground-url"); if playground.is_some() { markdown::PLAYGROUND_KRATE.with(|s| { *s.borrow_mut() = Some(None); }); @@ -144,7 +148,11 @@ pub fn render(input: &str, mut output: PathBuf, matches: &getopts::Matches, /// Run any tests/code examples in the markdown file `input`. pub fn test(input: &str, cfgs: Vec, libs: SearchPaths, externs: Externs, mut test_args: Vec) -> isize { - let input_str = load_or_return!(input, 1, 2); + let input_str = match load_string(input) { + Ok(s) => s, + Err(LoadStringError::ReadFail) => return 1, + Err(LoadStringError::BadUtf8) => return 2, + }; let mut opts = TestOptions::default(); opts.no_crate_inject = true; From ba20da1db760d9b21b6dde4388e8d03f2dd137f3 Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Sat, 8 Oct 2016 23:53:57 -0400 Subject: [PATCH 4/5] Make `ExternalHtml::load` short-circuited. --- src/librustdoc/externalfiles.rs | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index 79e30b0d6aad3..6a38f96671c59 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -30,16 +30,22 @@ pub struct ExternalHtml{ impl ExternalHtml { pub fn load(in_header: &[String], before_content: &[String], after_content: &[String]) -> Option { - match (load_external_files(in_header), - load_external_files(before_content), - load_external_files(after_content)) { - (Some(ih), Some(bc), Some(ac)) => Some(ExternalHtml { - in_header: ih, - before_content: bc, - after_content: ac - }), - _ => None - } + load_external_files(in_header) + .and_then(|ih| + load_external_files(before_content) + .map(|bc| (ih, bc)) + ) + .and_then(|(ih, bc)| + load_external_files(after_content) + .map(|ac| (ih, bc, ac)) + ) + .map(|(ih, bc, ac)| + ExternalHtml { + in_header: ih, + before_content: bc, + after_content: ac, + } + ) } } From e4f066fe8b189d1459580d41b792347ba5c371ef Mon Sep 17 00:00:00 2001 From: Corey Farwell Date: Sun, 9 Oct 2016 00:15:42 -0400 Subject: [PATCH 5/5] Remove unnecessary `pub` function classifier. --- src/librustdoc/externalfiles.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index 6a38f96671c59..d78f00497ca55 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -76,7 +76,7 @@ pub fn load_string>(file_path: P) -> Result Option { +fn load_external_files(names: &[String]) -> Option { let mut out = String::new(); for name in names { let s = match load_string(name) {