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.

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

rustdoc: do not use plain summary for trait impls #73819

Merged
merged 2 commits into from
Sep 3, 2020
Merged
Show file tree
Hide file tree
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
2 changes: 0 additions & 2 deletions library/core/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,6 @@ pub trait Write {
/// This method should generally not be invoked manually, but rather through
/// the [`write!`] macro itself.
///
/// [`write!`]: ../../std/macro.write.html
///
/// # Examples
///
/// ```
Expand Down
4 changes: 2 additions & 2 deletions library/core/src/iter/traits/double_ended.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ pub trait DoubleEndedIterator: Iterator {
/// This is the reverse version of [`try_fold()`]: it takes elements
/// starting from the back of the iterator.
///
/// [`try_fold()`]: trait.Iterator.html#method.try_fold
/// [`try_fold()`]: Iterator::try_fold
///
/// # Examples
///
Expand Down Expand Up @@ -214,7 +214,7 @@ pub trait DoubleEndedIterator: Iterator {
/// Folding is useful whenever you have a collection of something, and want
/// to produce a single value from it.
///
/// [`fold()`]: trait.Iterator.html#method.fold
/// [`fold()`]: Iterator::fold
///
/// # Examples
///
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2713,7 +2713,7 @@ pub trait Iterator {
/// This is useful when you have an iterator over `&T`, but you need an
/// iterator over `T`.
///
/// [`clone`]: crate::clone::Clone::clone
/// [`clone`]: Clone::clone
///
/// # Examples
///
Expand Down
4 changes: 2 additions & 2 deletions library/core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3219,7 +3219,7 @@ pub unsafe trait SliceIndex<T: ?Sized>: private_slice_index::Sealed {
/// Calling this method with an out-of-bounds index or a dangling `slice` pointer
/// is *[undefined behavior]* even if the resulting reference is not used.
///
/// [undefined behavior]: ../../reference/behavior-considered-undefined.html
/// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
#[unstable(feature = "slice_index_methods", issue = "none")]
unsafe fn get_unchecked(self, slice: *const T) -> *const Self::Output;

Expand All @@ -3228,7 +3228,7 @@ pub unsafe trait SliceIndex<T: ?Sized>: private_slice_index::Sealed {
/// Calling this method with an out-of-bounds index or a dangling `slice` pointer
/// is *[undefined behavior]* even if the resulting reference is not used.
///
/// [undefined behavior]: ../../reference/behavior-considered-undefined.html
/// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
#[unstable(feature = "slice_index_methods", issue = "none")]
unsafe fn get_unchecked_mut(self, slice: *mut T) -> *mut Self::Output;

Expand Down
6 changes: 3 additions & 3 deletions library/std/src/os/linux/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ pub trait MetadataExt {
fn st_atime(&self) -> i64;
/// Returns the last access time of the file, in nanoseconds since [`st_atime`].
///
/// [`st_atime`]: #tymethod.st_atime
/// [`st_atime`]: Self::st_atime
///
/// # Examples
///
Expand Down Expand Up @@ -232,7 +232,7 @@ pub trait MetadataExt {
fn st_mtime(&self) -> i64;
/// Returns the last modification time of the file, in nanoseconds since [`st_mtime`].
///
/// [`st_mtime`]: #tymethod.st_mtime
/// [`st_mtime`]: Self::st_mtime
///
/// # Examples
///
Expand Down Expand Up @@ -268,7 +268,7 @@ pub trait MetadataExt {
fn st_ctime(&self) -> i64;
/// Returns the last status change time of the file, in nanoseconds since [`st_ctime`].
///
/// [`st_ctime`]: #tymethod.st_ctime
/// [`st_ctime`]: Self::st_ctime
///
/// # Examples
///
Expand Down
6 changes: 3 additions & 3 deletions library/std/src/os/redox/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ pub trait MetadataExt {
fn st_atime(&self) -> i64;
/// Returns the last access time of the file, in nanoseconds since [`st_atime`].
///
/// [`st_atime`]: #tymethod.st_atime
/// [`st_atime`]: Self::st_atime
///
/// # Examples
///
Expand Down Expand Up @@ -236,7 +236,7 @@ pub trait MetadataExt {
fn st_mtime(&self) -> i64;
/// Returns the last modification time of the file, in nanoseconds since [`st_mtime`].
///
/// [`st_mtime`]: #tymethod.st_mtime
/// [`st_mtime`]: Self::st_mtime
///
/// # Examples
///
Expand Down Expand Up @@ -272,7 +272,7 @@ pub trait MetadataExt {
fn st_ctime(&self) -> i64;
/// Returns the last status change time of the file, in nanoseconds since [`st_ctime`].
///
/// [`st_ctime`]: #tymethod.st_ctime
/// [`st_ctime`]: Self::st_ctime
///
/// # Examples
///
Expand Down
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('`');
jyn514 marked this conversation as resolved.
Show resolved Hide resolved
}
Event::HardBreak | Event::SoftBreak => s.push(' '),
Event::Start(Tag::CodeBlock(..)) => break,
Event::End(Tag::Paragraph) => break,
_ => (),
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
}
}
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') {
Copy link
Member

Choose a reason for hiding this comment

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

Does it take into account when you only have an indented code block like:

///    hello

?

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'
Loading