Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add support for image, rules and footnotes #40919

Merged
merged 5 commits into from
Apr 3, 2017
Merged

Add support for image, rules and footnotes #40919

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
08a741e
Add support for image, rules and footnotes
GuillaumeGomez Mar 30, 2017
36b15f0
Fix multiple footnotes and improve testing
GuillaumeGomez Mar 30, 2017
51d3cec
Fix hard break issue
GuillaumeGomez Mar 31, 2017
4de4a95
Add end whitespace ignore flag for tidy
GuillaumeGomez Mar 31, 2017
ef01ae7
Force footnote references to be sorted by id
GuillaumeGomez Apr 1, 2017
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
139 changes: 111 additions & 28 deletions src/librustdoc/html/markdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,37 @@ macro_rules! event_loop_break {
}}
}

struct ParserWrapper<'a> {
parser: Parser<'a>,
footnotes: Vec<String>,
current_footnote_id: 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: Vec::new(),
current_footnote_id: 1,
}
}
pub fn next(&mut self) -> Option<Event<'a>> {
self.parser.next()
}

pub fn get_next_footnote_id(&mut self) -> u16 {
let tmp = self.current_footnote_id;
self.current_footnote_id += 1;
tmp
}
}

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) {
let mut origtext = String::new();
while let Some(event) = parser.next() {
match event {
Expand Down Expand Up @@ -215,8 +241,8 @@ 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) {
let mut ret = String::new();
let mut id = String::new();
event_loop_break!(parser, toc_builder, shorter, ret, true, &mut Some(&mut id),
Expand Down Expand Up @@ -249,32 +275,48 @@ 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>) {
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>) {
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>) {
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>) {
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) {
let mut content = String::new();
event_loop_break!(parser, toc_builder, shorter, content, true, &mut None,
Event::End(Tag::TableHead) |
Expand All @@ -284,8 +326,8 @@ 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) {
let mut content = String::new();
while let Some(event) = parser.next() {
match event {
Expand All @@ -303,8 +345,8 @@ 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) {
let mut content = String::new();
while let Some(event) = parser.next() {
match event {
Expand All @@ -322,7 +364,7 @@ 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) {
let mut content = String::new();
let mut rows = String::new();
Expand All @@ -347,16 +389,16 @@ 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) {
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) {
let mut content = String::new();
while let Some(event) = parser.next() {
match event {
Expand All @@ -372,7 +414,7 @@ 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) {
let mut content = String::new();
while let Some(event) = parser.next() {
Expand All @@ -389,23 +431,40 @@ 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>) {
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>) {
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,
mut definition: String, id: &mut Option<&mut String>) {
event_loop_break!(parser, toc_builder, shorter, definition, true, id,
Event::End(Tag::FootnoteDefinition(_)));
buffer.push_str(&definition);
}

fn rule(parser: &mut ParserWrapper, buffer: &mut String, toc_builder: &mut Option<TocBuilder>,
shorter: MarkdownOutputStyle, id: &mut Option<&mut String>) {
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 {
Expand All @@ -423,7 +482,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);
Expand All @@ -440,6 +502,23 @@ 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)) => {
let mut content = String::new();
footnote(parser, &mut content, toc_builder, shorter, def.as_ref().to_owned(),
id);
let cur_len = parser.footnotes.len() + 1;
parser.footnotes.push(format!("<li id=\"ref{}\">{}<a href=\"#supref{0}\" \
rev=\"footnote\">↩</a></li>",
Copy link

Choose a reason for hiding this comment

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

Where is parent ul generated?

Copy link
Member Author

Choose a reason for hiding this comment

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

At the end.

cur_len, content));
Copy link

Choose a reason for hiding this comment

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

This is wrong. For example:

foot[^note1], foot[^note2]

[^note1]: text1

[^note2]: text2

Will give the following event sequence:

[
    Start(Paragraph),
    Text("foot"),
    FootnoteReference("note1"),
    Text(", foot"),
    FootnoteReference("note2"),
    End(Paragraph),
    Start(FootnoteDefinition("note1")),
    Start(Paragraph),
    Text("text1"),
    End(Paragraph),
    End(FootnoteDefinition("note1"))
    Start(FootnoteDefinition("note2")),
    Start(Paragraph),
    Text("text2"),
    End(Paragraph),
    End(FootnoteDefinition("note2"))
]

Correct me if I am wrong, but according to this code snippet, both footnote definitions will have the same id cur_len.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh absolutely! Thanks for notifying it!

}
Event::FootnoteReference(_) => {
buffer.push_str(&format!("<sup id=\"supref{0}\"><a href=\"#ref{0}\">{0}</a>\
</sup>",
parser.get_next_footnote_id()));
Copy link

Choose a reason for hiding this comment

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

Footnote ids cannot be this simple either. The same footnote can be referenced from multiple places. So you can't simply keep incrementing the id whenever a footnote reference is found. You'd need to use a map.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, I'll update to this.

}
Event::Html(h) | Event::InlineHtml(h) => {
buffer.push_str(&*h);
}
Expand All @@ -457,13 +536,17 @@ 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() {
buffer.push_str(&format!("<div class=\"footnotes\"><hr><ol>{}</ol></div>",
parser.footnotes.join("")));
}
let mut ret = toc_builder.map_or(Ok(()), |builder| {
write!(w, "<nav id=\"TOC\">{}</nav>", builder.into_toc())
});
Expand Down
35 changes: 35 additions & 0 deletions src/test/rustdoc/check-rule-image-footnote.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// 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"]

// @has foo/fn.f.html
// @has - '<p>hard break: after hard break</p><hr>'
// @has - '<img src="https://www.rust-lang.org/logos/rust-logo-128x128-blk-v2.png" alt="Rust">'
// @has - '<li id="ref1">'
// @has - '<sup id="supref1"><a href="#ref1">1</a></sup>'
/// markdown test
///
/// this is a [link].
///
/// [link]: https://example.com "this is a title"
///
/// hard break:
/// after hard break
Copy link

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].
///
/// [^footnote]: Thing
///
/// ![Rust](https://www.rust-lang.org/logos/rust-logo-128x128-blk-v2.png)
#[deprecated(note = "Struct<T>")]
pub fn f() {}