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

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 4 commits
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
187 changes: 157 additions & 30 deletions src/librustdoc/html/markdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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 {
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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) |
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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() {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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);
Expand All @@ -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
}));
Copy link

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);
}
_ => {}
Expand All @@ -457,13 +577,20 @@ 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.values()
.map(|&(ref s, _)| s.as_str())
.collect::<Vec<_>>()
.join("")));
}
let mut ret = toc_builder.map_or(Ok(()), |builder| {
write!(w, "<nav id=\"TOC\">{}</nav>", builder.into_toc())
});
Expand Down
13 changes: 12 additions & 1 deletion src/librustdoc/passes/unindent_comments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
View file
Open in desktop
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() {}
Loading