Skip to content

Commit

Permalink
rustdoc: do not use plain summary for trait impls
Browse files Browse the repository at this point in the history
Fixes #38386.
Fixes #48332.
Fixes #49430.
Fixes #62741.
Fixes #73474.
  • Loading branch information
euclio committed Aug 30, 2020
1 parent db534b3 commit e0822ec
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 70 deletions.
4 changes: 2 additions & 2 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::formats::item_type::ItemType;
use crate::formats::Impl;
use crate::html::render::cache::{extern_location, get_index_search_type, ExternalLocation};
use crate::html::render::IndexItem;
use crate::html::render::{plain_summary_line, shorten};
use crate::html::render::{plain_text_summary, shorten};

thread_local!(crate static CACHE_KEY: RefCell<Arc<Cache>> = Default::default());

Expand Down Expand Up @@ -313,7 +313,7 @@ impl DocFolder for Cache {
ty: item.type_(),
name: s.to_string(),
path: path.join("::"),
desc: shorten(plain_summary_line(item.doc_value())),
desc: shorten(plain_text_summary(item.doc_value())),
parent,
parent_idx: None,
search_type: get_index_search_type(&item),
Expand Down
55 changes: 22 additions & 33 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -955,44 +955,33 @@ impl MarkdownSummaryLine<'_> {
}
}

pub fn plain_summary_line(md: &str) -> String {
struct ParserWrapper<'a> {
inner: Parser<'a>,
is_in: isize,
is_first: bool,
/// Renders the first paragraph of the provided markdown as plain text.
///
/// - Headings, links, and formatting are stripped.
/// - Inline code is rendered as-is, surrounded by backticks.
/// - HTML and code blocks are ignored.
pub fn plain_text_summary(md: &str) -> String {
if md.is_empty() {
return String::new();
}

impl<'a> Iterator for ParserWrapper<'a> {
type Item = String;

fn next(&mut self) -> Option<String> {
let next_event = self.inner.next()?;
let (ret, is_in) = match next_event {
Event::Start(Tag::Paragraph) => (None, 1),
Event::Start(Tag::Heading(_)) => (None, 1),
Event::Code(code) => (Some(format!("`{}`", code)), 0),
Event::Text(ref s) if self.is_in > 0 => (Some(s.as_ref().to_owned()), 0),
Event::End(Tag::Paragraph | Tag::Heading(_)) => (None, -1),
_ => (None, 0),
};
if is_in > 0 || (is_in < 0 && self.is_in > 0) {
self.is_in += is_in;
}
if ret.is_some() {
self.is_first = false;
ret
} else {
Some(String::new())
let mut s = String::with_capacity(md.len() * 3 / 2);

for event in Parser::new_ext(md, Options::ENABLE_STRIKETHROUGH) {
match &event {
Event::Text(text) => s.push_str(text),
Event::Code(code) => {
s.push('`');
s.push_str(code);
s.push('`');
}
Event::HardBreak | Event::SoftBreak => s.push(' '),
Event::Start(Tag::CodeBlock(..)) => break,
Event::End(Tag::Paragraph) => break,
_ => (),
}
}
let mut s = String::with_capacity(md.len() * 3 / 2);
let p = ParserWrapper {
inner: Parser::new_ext(md, Options::ENABLE_STRIKETHROUGH),
is_in: 0,
is_first: true,
};
p.filter(|t| !t.is_empty()).for_each(|i| s.push_str(&i));

s
}

Expand Down
13 changes: 10 additions & 3 deletions src/librustdoc/html/markdown/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::plain_summary_line;
use super::plain_text_summary;
use super::{ErrorCodes, IdMap, Ignore, LangString, Markdown, MarkdownHtml};
use rustc_span::edition::{Edition, DEFAULT_EDITION};
use std::cell::RefCell;
Expand Down Expand Up @@ -210,18 +210,25 @@ fn test_header_ids_multiple_blocks() {
}

#[test]
fn test_plain_summary_line() {
fn test_plain_text_summary() {
fn t(input: &str, expect: &str) {
let output = plain_summary_line(input);
let output = plain_text_summary(input);
assert_eq!(output, expect, "original: {}", input);
}

t("hello [Rust](https://www.rust-lang.org) :)", "hello Rust :)");
t("**bold**", "bold");
t("Multi-line\nsummary", "Multi-line summary");
t("Hard-break \nsummary", "Hard-break summary");
t("hello [Rust] :)\n\n[Rust]: https://www.rust-lang.org", "hello Rust :)");
t("hello [Rust](https://www.rust-lang.org \"Rust\") :)", "hello Rust :)");
t("code `let x = i32;` ...", "code `let x = i32;` ...");
t("type `Type<'static>` ...", "type `Type<'static>` ...");
t("# top header", "top header");
t("## header", "header");
t("first paragraph\n\nsecond paragraph", "first paragraph");
t("```\nfn main() {}\n```", "");
t("<div>hello</div>", "");
}

#[test]
Expand Down
6 changes: 3 additions & 3 deletions src/librustdoc/html/render/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::clean::types::GetDefId;
use crate::clean::{self, AttributesExt};
use crate::formats::cache::Cache;
use crate::formats::item_type::ItemType;
use crate::html::render::{plain_summary_line, shorten};
use crate::html::render::{plain_text_summary, shorten};
use crate::html::render::{Generic, IndexItem, IndexItemFunctionType, RenderType, TypeWithKind};

/// Indicates where an external crate can be found.
Expand Down Expand Up @@ -78,7 +78,7 @@ pub fn build_index(krate: &clean::Crate, cache: &mut Cache) -> String {
ty: item.type_(),
name: item.name.clone().unwrap(),
path: fqp[..fqp.len() - 1].join("::"),
desc: shorten(plain_summary_line(item.doc_value())),
desc: shorten(plain_text_summary(item.doc_value())),
parent: Some(did),
parent_idx: None,
search_type: get_index_search_type(&item),
Expand Down Expand Up @@ -127,7 +127,7 @@ pub fn build_index(krate: &clean::Crate, cache: &mut Cache) -> String {
let crate_doc = krate
.module
.as_ref()
.map(|module| shorten(plain_summary_line(module.doc_value())))
.map(|module| shorten(plain_text_summary(module.doc_value())))
.unwrap_or(String::new());

#[derive(Serialize)]
Expand Down
62 changes: 33 additions & 29 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1506,6 +1506,7 @@ impl Context {
}
}

/// Construct a map of items shown in the sidebar to a plain-text summary of their docs.
fn build_sidebar_items(&self, m: &clean::Module) -> BTreeMap<String, Vec<NameDoc>> {
// BTreeMap instead of HashMap to get a sorted output
let mut map: BTreeMap<_, Vec<_>> = BTreeMap::new();
Expand All @@ -1522,7 +1523,7 @@ impl Context {
let short = short.to_string();
map.entry(short)
.or_default()
.push((myname, Some(plain_summary_line(item.doc_value()))));
.push((myname, Some(plain_text_summary(item.doc_value()))));
}

if self.shared.sort_modules_alphabetically {
Expand Down Expand Up @@ -1728,22 +1729,15 @@ fn full_path(cx: &Context, item: &clean::Item) -> String {
s
}

/// Renders the first paragraph of the given markdown as plain text, making it suitable for
/// contexts like alt-text or the search index.
///
/// If no markdown is supplied, the empty string is returned.
///
/// See [`markdown::plain_text_summary`] for further details.
#[inline]
crate fn plain_summary_line(s: Option<&str>) -> String {
let s = s.unwrap_or("");
// This essentially gets the first paragraph of text in one line.
let mut line = s
.lines()
.skip_while(|line| line.chars().all(|c| c.is_whitespace()))
.take_while(|line| line.chars().any(|c| !c.is_whitespace()))
.fold(String::new(), |mut acc, line| {
acc.push_str(line);
acc.push(' ');
acc
});
// remove final whitespace
line.pop();
markdown::plain_summary_line(&line[..])
crate fn plain_text_summary(s: Option<&str>) -> String {
s.map(markdown::plain_text_summary).unwrap_or_default()
}

crate fn shorten(s: String) -> String {
Expand Down Expand Up @@ -1800,25 +1794,35 @@ fn render_markdown(
)
}

/// Writes a documentation block containing only the first paragraph of the documentation. If the
/// docs are longer, a "Read more" link is appended to the end.
fn document_short(
w: &mut Buffer,
cx: &Context,
item: &clean::Item,
link: AssocItemLink<'_>,
prefix: &str,
is_hidden: bool,
) {
if let Some(s) = item.doc_value() {
let markdown = if s.contains('\n') {
format!(
"{} [Read more]({})",
&plain_summary_line(Some(s)),
naive_assoc_href(item, link)
)
} else {
plain_summary_line(Some(s))
};
render_markdown(w, cx, &markdown, item.links(), prefix, is_hidden);
let mut summary_html = MarkdownSummaryLine(s, &item.links()).into_string();

if s.contains('\n') {
let link = format!(r#" <a href="{}">Read more</a>"#, naive_assoc_href(item, link));

if let Some(idx) = summary_html.rfind("</p>") {
summary_html.insert_str(idx, &link);
} else {
summary_html.push_str(&link);
}
}

write!(
w,
"<div class='docblock{}'>{}{}</div>",
if is_hidden { " hidden" } else { "" },
prefix,
summary_html,
);
} else if !prefix.is_empty() {
write!(
w,
Expand Down Expand Up @@ -3689,7 +3693,7 @@ fn render_impl(
} else if show_def_docs {
// In case the item isn't documented,
// provide short documentation from the trait.
document_short(w, cx, it, link, "", is_hidden);
document_short(w, it, link, "", is_hidden);
}
}
} else {
Expand All @@ -3701,7 +3705,7 @@ fn render_impl(
} else {
document_stability(w, cx, item, is_hidden);
if show_def_docs {
document_short(w, cx, item, link, "", is_hidden);
document_short(w, item, link, "", is_hidden);
}
}
}
Expand Down
26 changes: 26 additions & 0 deletions src/test/rustdoc/plain-text-summaries.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#![crate_type = "lib"]
#![crate_name = "summaries"]

//! This summary has a [link] and `code`.
//!
//! This is the second paragraph.
//!
//! [link]: https://example.com
// @has search-index.js 'This summary has a link and `code`.'
// @!has - 'second paragraph'

/// This `code` should be in backticks.
///
/// This text should not be rendered.
pub struct Sidebar;

// @has summaries/sidebar-items.js 'This `code` should be in backticks.'
// @!has - 'text should not be rendered'

/// ```text
/// this block should not be rendered
/// ```
pub struct Sidebar2;

// @!has summaries/sidebar-items.js 'block should not be rendered'
46 changes: 46 additions & 0 deletions src/test/rustdoc/trait-impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
pub trait Trait {
/// Some long docs here.
///
/// These docs are long enough that a link will be added to the end.
fn a();

/// These docs contain a [reference link].
///
/// [reference link]: https://example.com
fn b();

/// ```
/// This code block should not be in the output, but a Read more link should be generated
/// ```
fn c();

/// Escaped formatting a\*b\*c\* works
fn d();
}

pub struct Struct;

impl Trait for Struct {
// @has trait_impl/struct.Struct.html '//*[@id="method.a"]/../div/p' 'Some long docs'
// @!has - '//*[@id="method.a"]/../div/p' 'link will be added'
// @has - '//*[@id="method.a"]/../div/p/a' 'Read more'
// @has - '//*[@id="method.a"]/../div/p/a/@href' 'trait.Trait.html'
fn a() {}

// @has trait_impl/struct.Struct.html '//*[@id="method.b"]/../div/p' 'These docs contain'
// @has - '//*[@id="method.b"]/../div/p/a' 'reference link'
// @has - '//*[@id="method.b"]/../div/p/a/@href' 'https://example.com'
// @has - '//*[@id="method.b"]/../div/p/a' 'Read more'
// @has - '//*[@id="method.b"]/../div/p/a/@href' 'trait.Trait.html'
fn b() {}

// @!has trait_impl/struct.Struct.html '//*[@id="method.c"]/../div/p' 'code block'
// @has - '//*[@id="method.c"]/../div/p/a' 'Read more'
// @has - '//*[@id="method.c"]/../div/p/a/@href' 'trait.Trait.html'
fn c() {}

// @has trait_impl/struct.Struct.html '//*[@id="method.d"]/../div/p' \
// 'Escaped formatting a*b*c* works'
// @!has trait_impl/struct.Struct.html '//*[@id="method.d"]/../div/p/em'
fn d() {}
}

0 comments on commit e0822ec

Please sign in to comment.