From c250b5fd033ebd8257cca8ee537e752355a151c3 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 18:27:17 -0400 Subject: [PATCH] Remove fmt::Display impls on Markdown structs These impls prevent ergonomic use of the config (e.g., forcing us to use RefCell) despite all usecases for these structs only using their Display impls once. --- src/librustdoc/externalfiles.rs | 4 +- src/librustdoc/html/markdown.rs | 57 +++++++++++-------------- src/librustdoc/html/render.rs | 10 ++--- src/tools/error_index_generator/main.rs | 2 +- 4 files changed, 34 insertions(+), 39 deletions(-) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index d920b7c4c916..5d953eec31ec 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -36,7 +36,7 @@ impl ExternalHtml { load_external_files(md_before_content, diag) .map(|m_bc| (ih, format!("{}{}", bc, Markdown(&m_bc, &[], RefCell::new(id_map), - codes, edition, playground)))) + codes, edition, playground).to_string()))) ) .and_then(|(ih, bc)| load_external_files(after_content, diag) @@ -46,7 +46,7 @@ impl ExternalHtml { load_external_files(md_after_content, diag) .map(|m_ac| (ih, bc, format!("{}{}", ac, Markdown(&m_ac, &[], RefCell::new(id_map), - codes, edition, playground)))) + codes, edition, playground).to_string()))) ) .map(|(ih, bc, ac)| ExternalHtml { diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 73233a2289cc..753832305f9e 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1,9 +1,6 @@ //! Markdown formatting for rustdoc. //! -//! This module implements markdown formatting through the pulldown-cmark -//! rust-library. This module exposes all of the -//! functionality through a unit struct, `Markdown`, which has an implementation -//! of `fmt::Display`. Example usage: +//! This module implements markdown formatting through the pulldown-cmark library. //! //! ``` //! #![feature(rustc_private)] @@ -16,8 +13,9 @@ //! //! let s = "My *markdown* _text_"; //! let mut id_map = IdMap::new(); -//! let html = format!("{}", Markdown(s, &[], RefCell::new(&mut id_map), -//! ErrorCodes::Yes, Edition::Edition2015, None)); +//! let md = Markdown(s, &[], RefCell::new(&mut id_map), +//! ErrorCodes::Yes, Edition::Edition2015, None); +//! let html = md.to_string(); //! // ... something using html //! ``` @@ -27,7 +25,7 @@ use rustc_data_structures::fx::FxHashMap; use std::cell::RefCell; use std::collections::VecDeque; use std::default::Default; -use std::fmt::{self, Write}; +use std::fmt::Write; use std::borrow::Cow; use std::ops::Range; use std::str; @@ -46,9 +44,8 @@ fn opts() -> Options { Options::ENABLE_TABLES | Options::ENABLE_FOOTNOTES } -/// A tuple struct that has the `fmt::Display` trait implemented. -/// When formatted, this struct will emit the HTML corresponding to the rendered -/// version of the contained markdown string. +/// When `to_string` is called, this struct will emit the HTML corresponding to +/// the rendered version of the contained markdown string. pub struct Markdown<'a>( pub &'a str, /// A list of link replacements. @@ -691,13 +688,13 @@ impl LangString { } } -impl<'a> fmt::Display for Markdown<'a> { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let Markdown(md, links, ref ids, codes, edition, playground) = *self; +impl Markdown<'_> { + pub fn to_string(self) -> String { + let Markdown(md, links, ids, codes, edition, playground) = self; let mut ids = ids.borrow_mut(); // This is actually common enough to special-case - if md.is_empty() { return Ok(()) } + if md.is_empty() { return String::new(); } let replacer = |_: &str, s: &str| { if let Some(&(_, ref replace)) = links.into_iter().find(|link| &*link.0 == s) { Some((replace.clone(), s.to_owned())) @@ -716,13 +713,13 @@ impl<'a> fmt::Display for Markdown<'a> { let p = Footnotes::new(p); html::push_html(&mut s, p); - fmt.write_str(&s) + s } } -impl<'a> fmt::Display for MarkdownWithToc<'a> { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let MarkdownWithToc(md, ref ids, codes, edition, playground) = *self; +impl MarkdownWithToc<'_> { + pub fn to_string(self) -> String { + let MarkdownWithToc(md, ref ids, codes, edition, playground) = self; let mut ids = ids.borrow_mut(); let p = Parser::new_ext(md, opts()); @@ -738,19 +735,17 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> { html::push_html(&mut s, p); } - write!(fmt, "", toc.into_toc())?; - - fmt.write_str(&s) + format!("{}", toc.into_toc(), s) } } -impl<'a> fmt::Display for MarkdownHtml<'a> { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let MarkdownHtml(md, ref ids, codes, edition, playground) = *self; +impl MarkdownHtml<'_> { + pub fn to_string(self) -> String { + let MarkdownHtml(md, ref ids, codes, edition, playground) = self; let mut ids = ids.borrow_mut(); // This is actually common enough to special-case - if md.is_empty() { return Ok(()) } + if md.is_empty() { return String::new(); } let p = Parser::new_ext(md, opts()); // Treat inline HTML as plain text. @@ -766,15 +761,15 @@ impl<'a> fmt::Display for MarkdownHtml<'a> { let p = Footnotes::new(p); html::push_html(&mut s, p); - fmt.write_str(&s) + s } } -impl<'a> fmt::Display for MarkdownSummaryLine<'a> { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let MarkdownSummaryLine(md, links) = *self; +impl MarkdownSummaryLine<'_> { + pub fn to_string(self) -> String { + let MarkdownSummaryLine(md, links) = self; // This is actually common enough to special-case - if md.is_empty() { return Ok(()) } + if md.is_empty() { return String::new(); } let replacer = |_: &str, s: &str| { if let Some(&(_, ref replace)) = links.into_iter().find(|link| &*link.0 == s) { @@ -790,7 +785,7 @@ impl<'a> fmt::Display for MarkdownSummaryLine<'a> { html::push_html(&mut s, LinkReplacer::new(SummaryLine::new(p), links)); - fmt.write_str(&s) + s } } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 8a7dfebdbbc1..fc4e25e3d7ff 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -2596,7 +2596,7 @@ fn render_markdown(w: &mut fmt::Formatter<'_>, if is_hidden { " hidden" } else { "" }, prefix, Markdown(md_text, &links, RefCell::new(&mut ids), - cx.codes, cx.edition, &cx.playground)) + cx.codes, cx.edition, &cx.playground).to_string()) } fn document_short( @@ -2866,7 +2866,7 @@ fn item_module(w: &mut fmt::Formatter<'_>, cx: &Context, ", name = *myitem.name.as_ref().unwrap(), stab_tags = stability_tags(myitem), - docs = MarkdownSummaryLine(doc_value, &myitem.links()), + docs = MarkdownSummaryLine(doc_value, &myitem.links()).to_string(), class = myitem.type_(), add = add, stab = stab.unwrap_or_else(|| String::new()), @@ -2963,7 +2963,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { let mut ids = cx.id_map.borrow_mut(); let html = MarkdownHtml( ¬e, RefCell::new(&mut ids), error_codes, cx.edition, &cx.playground); - message.push_str(&format!(": {}", html)); + message.push_str(&format!(": {}", html.to_string())); } stability.push(format!("
{}
", message)); } @@ -3017,7 +3017,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { error_codes, cx.edition, &cx.playground, - ) + ).to_string() ); } @@ -4248,7 +4248,7 @@ fn render_impl(w: &mut fmt::Formatter<'_>, cx: &Context, i: &Impl, link: AssocIt let mut ids = cx.id_map.borrow_mut(); write!(w, "
{}
", Markdown(&*dox, &i.impl_item.links(), RefCell::new(&mut ids), - cx.codes, cx.edition, &cx.playground))?; + cx.codes, cx.edition, &cx.playground).to_string())?; } } diff --git a/src/tools/error_index_generator/main.rs b/src/tools/error_index_generator/main.rs index 33987b0b5421..89b545fb7e41 100644 --- a/src/tools/error_index_generator/main.rs +++ b/src/tools/error_index_generator/main.rs @@ -101,7 +101,7 @@ impl Formatter for HTMLFormatter { }; write!(output, "{}", Markdown(desc, &[], RefCell::new(&mut id_map), - ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)))? + ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)).to_string())? }, None => write!(output, "

No description.

\n")?, }