Skip to content

Add support for image, rules and footnotes #40919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 3, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 159 additions & 30 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
@@ -27,6 +27,7 @@

use std::ascii::AsciiExt;
use std::cell::RefCell;
use std::collections::HashMap;
use std::default::Default;
use std::fmt::{self, Write};
use std::str;
@@ -115,15 +116,19 @@ macro_rules! event_loop_break {
match event {
$($end_event)|* => break,
Event::Text(ref s) => {
debug!("Text");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this meant to be left in?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not opposed to keeping the debugging statements, but it'd be nicer if there were a little more descriptive, maybe something like debug!("markdown parser encountered 'Text'"), whatever works though.

inner($id, s);
if $escape {
$buf.push_str(&format!("{}", Escape(s)));
} else {
$buf.push_str(s);
}
}
Event::SoftBreak | Event::HardBreak if !$buf.is_empty() => {
$buf.push(' ');
Event::SoftBreak => {
debug!("SoftBreak");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

if !$buf.is_empty() {
$buf.push(' ');
}
}
x => {
looper($parser, &mut $buf, Some(x), $toc_builder, $shorter, $id);
@@ -133,11 +138,38 @@ macro_rules! event_loop_break {
}}
}

struct ParserWrapper<'a> {
parser: Parser<'a>,
// The key is the footnote reference. The value is the footnote definition and the id.
footnotes: HashMap<String, (String, u16)>,
}

impl<'a> ParserWrapper<'a> {
pub fn new(s: &'a str) -> ParserWrapper<'a> {
ParserWrapper {
parser: Parser::new_ext(s, pulldown_cmark::OPTION_ENABLE_TABLES |
pulldown_cmark::OPTION_ENABLE_FOOTNOTES),
footnotes: HashMap::new(),
}
}

pub fn next(&mut self) -> Option<Event<'a>> {
self.parser.next()
}

pub fn get_entry(&mut self, key: &str) -> &mut (String, u16) {
let new_id = self.footnotes.keys().count() + 1;
let key = key.to_owned();
self.footnotes.entry(key).or_insert((String::new(), new_id as u16))
}
}

pub fn render(w: &mut fmt::Formatter,
s: &str,
print_toc: bool,
shorter: MarkdownOutputStyle) -> fmt::Result {
fn code_block(parser: &mut Parser, buffer: &mut String, lang: &str) {
fn code_block(parser: &mut ParserWrapper, buffer: &mut String, lang: &str) {
debug!("CodeBlock");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I won't comment on all of these 😄

let mut origtext = String::new();
while let Some(event) = parser.next() {
match event {
@@ -215,8 +247,9 @@ pub fn render(w: &mut fmt::Formatter,
});
}

fn heading(parser: &mut Parser, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle, level: i32) {
fn heading(parser: &mut ParserWrapper, buffer: &mut String,
toc_builder: &mut Option<TocBuilder>, shorter: MarkdownOutputStyle, level: i32) {
debug!("Heading");
let mut ret = String::new();
let mut id = String::new();
event_loop_break!(parser, toc_builder, shorter, ret, true, &mut Some(&mut id),
@@ -249,32 +282,53 @@ pub fn render(w: &mut fmt::Formatter,
ret, lvl = level, id = id, sec = sec));
}

fn inline_code(parser: &mut Parser, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle, id: &mut Option<&mut String>) {
fn inline_code(parser: &mut ParserWrapper, buffer: &mut String,
toc_builder: &mut Option<TocBuilder>, shorter: MarkdownOutputStyle,
id: &mut Option<&mut String>) {
debug!("InlineCode");
let mut content = String::new();
event_loop_break!(parser, toc_builder, shorter, content, false, id, Event::End(Tag::Code));
buffer.push_str(&format!("<code>{}</code>",
Escape(&collapse_whitespace(content.trim_right()))));
}

fn link(parser: &mut Parser, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
fn link(parser: &mut ParserWrapper, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle, url: &str, title: &str,
id: &mut Option<&mut String>) {
debug!("Link");
let mut content = String::new();
event_loop_break!(parser, toc_builder, shorter, content, true, id,
Event::End(Tag::Link(_, _)));
if title.is_empty() {
buffer.push_str(&format!("<a href=\"{}\">{}</a>", url, content));
} else {
buffer.push_str(&format!("<a href=\"{}\" title=\"{}\">{}</a>",
url, Escape(title), content));
}
}

fn image(parser: &mut ParserWrapper, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle, url: &str, mut title: String,
id: &mut Option<&mut String>) {
debug!("Image");
event_loop_break!(parser, toc_builder, shorter, title, true, id,
Event::End(Tag::Link(_, _)));
buffer.push_str(&format!("<a href=\"{}\">{}</a>", url, title));
Event::End(Tag::Image(_, _)));
buffer.push_str(&format!("<img src=\"{}\" alt=\"{}\">", url, title));
}

fn paragraph(parser: &mut Parser, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle, id: &mut Option<&mut String>) {
fn paragraph(parser: &mut ParserWrapper, buffer: &mut String,
toc_builder: &mut Option<TocBuilder>, shorter: MarkdownOutputStyle,
id: &mut Option<&mut String>) {
debug!("Paragraph");
let mut content = String::new();
event_loop_break!(parser, toc_builder, shorter, content, true, id,
Event::End(Tag::Paragraph));
buffer.push_str(&format!("<p>{}</p>", content.trim_right()));
}

fn table_cell(parser: &mut Parser, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle) {
fn table_cell(parser: &mut ParserWrapper, buffer: &mut String,
toc_builder: &mut Option<TocBuilder>, shorter: MarkdownOutputStyle) {
debug!("TableCell");
let mut content = String::new();
event_loop_break!(parser, toc_builder, shorter, content, true, &mut None,
Event::End(Tag::TableHead) |
@@ -284,8 +338,9 @@ pub fn render(w: &mut fmt::Formatter,
buffer.push_str(&format!("<td>{}</td>", content.trim()));
}

fn table_row(parser: &mut Parser, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle) {
fn table_row(parser: &mut ParserWrapper, buffer: &mut String,
toc_builder: &mut Option<TocBuilder>, shorter: MarkdownOutputStyle) {
debug!("TableRow");
let mut content = String::new();
while let Some(event) = parser.next() {
match event {
@@ -303,8 +358,9 @@ pub fn render(w: &mut fmt::Formatter,
buffer.push_str(&format!("<tr>{}</tr>", content));
}

fn table_head(parser: &mut Parser, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle) {
fn table_head(parser: &mut ParserWrapper, buffer: &mut String,
toc_builder: &mut Option<TocBuilder>, shorter: MarkdownOutputStyle) {
debug!("TableHead");
let mut content = String::new();
while let Some(event) = parser.next() {
match event {
@@ -322,8 +378,9 @@ pub fn render(w: &mut fmt::Formatter,
}
}

fn table(parser: &mut Parser, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
fn table(parser: &mut ParserWrapper, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle) {
debug!("Table");
let mut content = String::new();
let mut rows = String::new();
while let Some(event) = parser.next() {
@@ -347,16 +404,18 @@ pub fn render(w: &mut fmt::Formatter,
}));
}

fn blockquote(parser: &mut Parser, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle) {
fn blockquote(parser: &mut ParserWrapper, buffer: &mut String,
toc_builder: &mut Option<TocBuilder>, shorter: MarkdownOutputStyle) {
debug!("BlockQuote");
let mut content = String::new();
event_loop_break!(parser, toc_builder, shorter, content, true, &mut None,
Event::End(Tag::BlockQuote));
buffer.push_str(&format!("<blockquote>{}</blockquote>", content.trim_right()));
}

fn list_item(parser: &mut Parser, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle) {
fn list_item(parser: &mut ParserWrapper, buffer: &mut String,
toc_builder: &mut Option<TocBuilder>, shorter: MarkdownOutputStyle) {
debug!("ListItem");
let mut content = String::new();
while let Some(event) = parser.next() {
match event {
@@ -372,8 +431,9 @@ pub fn render(w: &mut fmt::Formatter,
buffer.push_str(&format!("<li>{}</li>", content));
}

fn list(parser: &mut Parser, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
fn list(parser: &mut ParserWrapper, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle) {
debug!("List");
let mut content = String::new();
while let Some(event) = parser.next() {
match event {
@@ -389,23 +449,45 @@ pub fn render(w: &mut fmt::Formatter,
buffer.push_str(&format!("<ul>{}</ul>", content));
}

fn emphasis(parser: &mut Parser, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle, id: &mut Option<&mut String>) {
fn emphasis(parser: &mut ParserWrapper, buffer: &mut String,
toc_builder: &mut Option<TocBuilder>, shorter: MarkdownOutputStyle,
id: &mut Option<&mut String>) {
debug!("Emphasis");
let mut content = String::new();
event_loop_break!(parser, toc_builder, shorter, content, false, id,
Event::End(Tag::Emphasis));
buffer.push_str(&format!("<em>{}</em>", content));
}

fn strong(parser: &mut Parser, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
fn strong(parser: &mut ParserWrapper, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle, id: &mut Option<&mut String>) {
debug!("Strong");
let mut content = String::new();
event_loop_break!(parser, toc_builder, shorter, content, false, id,
Event::End(Tag::Strong));
buffer.push_str(&format!("<strong>{}</strong>", content));
}

fn looper<'a>(parser: &'a mut Parser, buffer: &mut String, next_event: Option<Event<'a>>,
fn footnote(parser: &mut ParserWrapper, buffer: &mut String,
toc_builder: &mut Option<TocBuilder>, shorter: MarkdownOutputStyle,
id: &mut Option<&mut String>) {
debug!("FootnoteDefinition");
let mut content = String::new();
event_loop_break!(parser, toc_builder, shorter, content, true, id,
Event::End(Tag::FootnoteDefinition(_)));
buffer.push_str(&content);
}

fn rule(parser: &mut ParserWrapper, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle, id: &mut Option<&mut String>) {
debug!("Rule");
let mut content = String::new();
event_loop_break!(parser, toc_builder, shorter, content, true, id,
Event::End(Tag::Rule));
buffer.push_str("<hr>");
}

fn looper<'a>(parser: &'a mut ParserWrapper, buffer: &mut String, next_event: Option<Event<'a>>,
toc_builder: &mut Option<TocBuilder>, shorter: MarkdownOutputStyle,
id: &mut Option<&mut String>) -> bool {
if let Some(event) = next_event {
@@ -423,7 +505,10 @@ pub fn render(w: &mut fmt::Formatter,
paragraph(parser, buffer, toc_builder, shorter, id);
}
Event::Start(Tag::Link(ref url, ref t)) => {
link(parser, buffer, toc_builder, shorter, url, t.as_ref().to_owned(), id);
link(parser, buffer, toc_builder, shorter, url, t.as_ref(), id);
}
Event::Start(Tag::Image(ref url, ref t)) => {
image(parser, buffer, toc_builder, shorter, url, t.as_ref().to_owned(), id);
}
Event::Start(Tag::Table(_)) => {
table(parser, buffer, toc_builder, shorter);
@@ -440,7 +525,42 @@ pub fn render(w: &mut fmt::Formatter,
Event::Start(Tag::Strong) => {
strong(parser, buffer, toc_builder, shorter, id);
}
Event::Start(Tag::Rule) => {
rule(parser, buffer, toc_builder, shorter, id);
}
Event::Start(Tag::FootnoteDefinition(ref def)) => {
debug!("FootnoteDefinition");
let mut content = String::new();
let def = def.as_ref();
footnote(parser, &mut content, toc_builder, shorter, id);
let entry = parser.get_entry(def);
let cur_id = (*entry).1;
(*entry).0.push_str(&format!("<li id=\"ref{}\">{}&nbsp;<a href=\"#supref{0}\" \
rev=\"footnote\">↩</a></p></li>",
cur_id,
if content.ends_with("</p>") {
&content[..content.len() - 4]
} else {
&content
}));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's one more issue here, though it would be very rarely encountered, if at all. When there are multiple footnote references, there needs to be multiple backreferences from the definition too, to be 100% proper. (For a nice reference implementation, see Wikipedia.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, for now let's keep it aside. This PR is already big enough.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(But it's a great suggestion!)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the current footnote implementation does this. I think @GuillaumeGomez is right though, we can add that back in in another PR; it's not as big of a deal as this stuff is.

}
Event::FootnoteReference(ref reference) => {
debug!("FootnoteReference");
let entry = parser.get_entry(reference.as_ref());
buffer.push_str(&format!("<sup id=\"supref{0}\"><a href=\"#ref{0}\">{0}</a>\
</sup>",
(*entry).1));
}
Event::HardBreak => {
debug!("HardBreak");
if shorter.is_fancy() {
buffer.push_str("<br>");
} else if !buffer.is_empty() {
buffer.push(' ');
}
}
Event::Html(h) | Event::InlineHtml(h) => {
debug!("Html/InlineHtml");
buffer.push_str(&*h);
}
_ => {}
@@ -457,13 +577,22 @@ pub fn render(w: &mut fmt::Formatter,
None
};
let mut buffer = String::new();
let mut parser = Parser::new_ext(s, pulldown_cmark::OPTION_ENABLE_TABLES);
let mut parser = ParserWrapper::new(s);
loop {
let next_event = parser.next();
if !looper(&mut parser, &mut buffer, next_event, &mut toc_builder, shorter, &mut None) {
break
}
}
if !parser.footnotes.is_empty() {
let mut v: Vec<_> = parser.footnotes.values().collect();
v.sort_by(|a, b| a.1.cmp(&b.1));
buffer.push_str(&format!("<div class=\"footnotes\"><hr><ol>{}</ol></div>",
v.iter()
.map(|s| s.0.as_str())
.collect::<Vec<_>>()
.join("")));
}
let mut ret = toc_builder.map_or(Ok(()), |builder| {
write!(w, "<nav id=\"TOC\">{}</nav>", builder.into_toc())
});
13 changes: 12 additions & 1 deletion src/librustdoc/passes/unindent_comments.rs
Original file line number Diff line number Diff line change
@@ -82,7 +82,7 @@ fn unindent(s: &str) -> String {
});

if !lines.is_empty() {
let mut unindented = vec![ lines[0].trim().to_string() ];
let mut unindented = vec![ lines[0].trim_left().to_string() ];
unindented.extend_from_slice(&lines[1..].iter().map(|&line| {
if line.chars().all(|c| c.is_whitespace()) {
line.to_string()
@@ -160,4 +160,15 @@ mod unindent_tests {
let r = unindent(&s);
assert_eq!(r, "line1\nline2");
}

#[test]
fn should_not_trim() {
let s = "\t line1 \n\t line2".to_string();
let r = unindent(&s);
assert_eq!(r, "line1 \nline2");

let s = " \tline1 \n \tline2".to_string();
let r = unindent(&s);
assert_eq!(r, "line1 \nline2");
}
}
19 changes: 19 additions & 0 deletions src/test/rustdoc/check-hard-break.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![crate_name = "foo"]

// ignore-tidy-end-whitespace

// @has foo/fn.f.html
// @has - '<p>hard break:<br>after hard break</p>'
/// hard break:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a review note; this works because there are the two trailing spaces. I was confused at first 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the point of hard break. :p

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I just meant it's hard to see in this diff.

/// after hard break
pub fn f() {}
40 changes: 40 additions & 0 deletions src/test/rustdoc/check-rule-image-footnote.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![crate_name = "foo"]

// ignore-tidy-linelength

// @has foo/fn.f.html
// @has - '<p>markdown test</p><p>this is a <a href="https://example.com" title="this is a title">link</a>.</p><p>hard break: after hard break</p><hr><p>a footnote<sup id="supref1"><a href="#ref1">1</a></sup>.</p><p>another footnote<sup id="supref2"><a href="#ref2">2</a></sup>.</p><p><img src="https://www.rust-lang.org/logos/rust-logo-128x128-blk-v2.png" alt="Rust"></p><div class="footnotes"><hr><ol><li id="ref1"><p>Thing&nbsp;<a href="#supref1" rev="footnote">↩</a></p></li><li id="ref2"><p>Another Thing&nbsp;<a href="#supref2" rev="footnote">↩</a></p></li></ol></div>'
/// markdown test
///
/// this is a [link].
///
/// [link]: https://example.com "this is a title"
///
/// hard break:
/// after hard break

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this generates a sequence like:

[..., Text("hard break:"), SoftBreak, Text("after hard break"), ...]

To generate a HardBreak in place of SoftBreak, there needs to be at least 2 trailing spaces right before the newline character:

"hard break  \nafter hard break"

ref

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is even before the markdown parsing, a trim is being done somewhere so the text the parser receives never has lines ending with whitespaces.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a test for it. Should be fully working now.

///
/// -----------
///
/// a footnote[^footnote].
///
/// another footnote[^footnotebis].
///
/// [^footnote]: Thing
///
///
/// [^footnotebis]: Another Thing
///
///
/// ![Rust](https://www.rust-lang.org/logos/rust-logo-128x128-blk-v2.png)
#[deprecated(note = "Struct<T>")]
pub fn f() {}
3 changes: 2 additions & 1 deletion src/tools/tidy/src/style.rs
Original file line number Diff line number Diff line change
@@ -110,6 +110,7 @@ pub fn check(path: &Path, bad: &mut bool) {
let skip_cr = contents.contains("ignore-tidy-cr");
let skip_tab = contents.contains("ignore-tidy-tab");
let skip_length = contents.contains("ignore-tidy-linelength");
let skip_end_whitespace = contents.contains("ignore-tidy-end-whitespace");
for (i, line) in contents.split("\n").enumerate() {
let mut err = |msg: &str| {
println!("{}:{}: {}", file.display(), i + 1, msg);
@@ -122,7 +123,7 @@ pub fn check(path: &Path, bad: &mut bool) {
if line.contains("\t") && !skip_tab {
err("tab character");
}
if line.ends_with(" ") || line.ends_with("\t") {
if !skip_end_whitespace && (line.ends_with(" ") || line.ends_with("\t")) {
err("trailing whitespace");
}
if line.contains("\r") && !skip_cr {