Skip to content

Commit

Permalink
Remove fmt::Display impls on Markdown structs
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Mark-Simulacrum committed Aug 11, 2019
1 parent dbad77f commit c250b5f
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 39 deletions.
4 changes: 2 additions & 2 deletions src/librustdoc/externalfiles.rs
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
57 changes: 26 additions & 31 deletions 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)]
Expand All @@ -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
//! ```

Expand All @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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()))
Expand All @@ -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());
Expand All @@ -738,19 +735,17 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> {
html::push_html(&mut s, p);
}

write!(fmt, "<nav id=\"TOC\">{}</nav>", toc.into_toc())?;

fmt.write_str(&s)
format!("<nav id=\"TOC\">{}</nav>{}", 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.
Expand All @@ -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) {
Expand All @@ -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
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/librustdoc/html/render.rs
Expand Up @@ -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(
Expand Down Expand Up @@ -2866,7 +2866,7 @@ fn item_module(w: &mut fmt::Formatter<'_>, cx: &Context,
</tr>",
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()),
Expand Down Expand Up @@ -2963,7 +2963,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec<String> {
let mut ids = cx.id_map.borrow_mut();
let html = MarkdownHtml(
&note, 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!("<div class='stab deprecated'>{}</div>", message));
}
Expand Down Expand Up @@ -3017,7 +3017,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec<String> {
error_codes,
cx.edition,
&cx.playground,
)
).to_string()
);
}

Expand Down Expand Up @@ -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, "<div class='docblock'>{}</div>",
Markdown(&*dox, &i.impl_item.links(), RefCell::new(&mut ids),
cx.codes, cx.edition, &cx.playground))?;
cx.codes, cx.edition, &cx.playground).to_string())?;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/tools/error_index_generator/main.rs
Expand Up @@ -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, "<p>No description.</p>\n")?,
}
Expand Down

0 comments on commit c250b5f

Please sign in to comment.