From 5d4a7128d9929723769a7c674f49e9fe3fc0ff13 Mon Sep 17 00:00:00 2001 From: Camelid Date: Sun, 4 Oct 2020 20:42:34 -0700 Subject: [PATCH 1/6] Render Markdown in search results Previously Markdown documentation was not rendered to HTML for search results, which led to the output not being very readable, particularly for inline code. This PR fixes that by rendering Markdown to HTML with the help of pulldown-cmark (the library rustdoc uses to parse Markdown for the main text of documentation). However, the text for the title attribute (the text shown when you hover over an element) still uses the plain-text rendering since it is displayed in browsers as plain-text. Only these styles will be rendered; everything else is stripped away: * *italics* * **bold** * `inline code` --- src/librustdoc/clean/mod.rs | 2 +- src/librustdoc/formats/cache.rs | 8 +- src/librustdoc/html/markdown.rs | 92 ++++++++++++++++++- src/librustdoc/html/markdown/tests.rs | 33 ++++++- src/librustdoc/html/render/cache.rs | 6 +- src/librustdoc/html/render/mod.rs | 41 ++------- src/librustdoc/html/static/main.js | 25 ++++- ...ext-summaries.rs => markdown-summaries.rs} | 9 +- 8 files changed, 166 insertions(+), 50 deletions(-) rename src/test/rustdoc/{plain-text-summaries.rs => markdown-summaries.rs} (55%) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index d294d8f02a8..40218022a19 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -1015,7 +1015,7 @@ impl<'tcx> Clean for (DefId, ty::PolyFnSig<'tcx>) { .iter() .map(|t| Argument { type_: t.clean(cx), - name: names.next().map_or(String::new(), |name| name.to_string()), + name: names.next().map_or_else(|| String::new(), |name| name.to_string()), }) .collect(), }, diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index c3153f2d4b6..e82bc540e95 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -14,13 +14,13 @@ use crate::config::RenderInfo; use crate::fold::DocFolder; use crate::formats::item_type::ItemType; use crate::formats::Impl; +use crate::html::markdown::short_markdown_summary; use crate::html::render::cache::{extern_location, get_index_search_type, ExternalLocation}; use crate::html::render::IndexItem; -use crate::html::render::{plain_text_summary, shorten}; thread_local!(crate static CACHE_KEY: RefCell> = Default::default()); -/// This cache is used to store information about the `clean::Crate` being +/// This cache is used to store information about the [`clean::Crate`] being /// rendered in order to provide more useful documentation. This contains /// information like all implementors of a trait, all traits a type implements, /// documentation for all known traits, etc. @@ -313,7 +313,9 @@ impl DocFolder for Cache { ty: item.type_(), name: s.to_string(), path: path.join("::"), - desc: shorten(plain_text_summary(item.doc_value())), + desc: item + .doc_value() + .map_or_else(|| String::new(), short_markdown_summary), parent, parent_idx: None, search_type: get_index_search_type(&item), diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 8ce686c6550..6bba5036191 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -17,8 +17,6 @@ //! // ... something using html //! ``` -#![allow(non_camel_case_types)] - use rustc_data_structures::fx::FxHashMap; use rustc_hir::def_id::DefId; use rustc_hir::HirId; @@ -1037,7 +1035,97 @@ impl MarkdownSummaryLine<'_> { } } +/// Renders a subset of Markdown in the first paragraph of the provided Markdown. +/// +/// - *Italics*, **bold**, and `inline code` styles **are** rendered. +/// - Headings and links are stripped (though the text *is* rendered). +/// - HTML, code blocks, and everything else are ignored. +/// +/// Returns a tuple of the rendered HTML string and whether the output was shortened +/// due to the provided `length_limit`. +fn markdown_summary_with_limit(md: &str, length_limit: Option) -> (String, bool) { + if md.is_empty() { + return (String::new(), false); + } + + let length_limit = length_limit.unwrap_or(u16::MAX) as usize; + + let mut s = String::with_capacity(md.len() * 3 / 2); + let mut text_length = 0; + let mut stopped_early = false; + + fn push(s: &mut String, text_length: &mut usize, text: &str) { + s.push_str(text); + *text_length += text.len(); + }; + + 'outer: for event in Parser::new_ext(md, Options::ENABLE_STRIKETHROUGH) { + match &event { + Event::Text(text) => { + for word in text.split_inclusive(char::is_whitespace) { + if text_length + word.len() >= length_limit { + stopped_early = true; + break 'outer; + } + + push(&mut s, &mut text_length, word); + } + } + Event::Code(code) => { + if text_length + code.len() >= length_limit { + stopped_early = true; + break; + } + + s.push_str(""); + push(&mut s, &mut text_length, code); + s.push_str(""); + } + Event::Start(tag) => match tag { + Tag::Emphasis => s.push_str(""), + Tag::Strong => s.push_str(""), + Tag::CodeBlock(..) => break, + _ => {} + }, + Event::End(tag) => match tag { + Tag::Emphasis => s.push_str(""), + Tag::Strong => s.push_str(""), + Tag::Paragraph => break, + _ => {} + }, + Event::HardBreak | Event::SoftBreak => { + if text_length + 1 >= length_limit { + stopped_early = true; + break; + } + + push(&mut s, &mut text_length, " "); + } + _ => {} + } + } + + (s, stopped_early) +} + +/// Renders a shortened first paragraph of the given Markdown as a subset of Markdown, +/// making it suitable for contexts like the search index. +/// +/// Will shorten to 59 or 60 characters, including an ellipsis (…) if it was shortened. +/// +/// See [`markdown_summary_with_limit`] for details about what is rendered and what is not. +crate fn short_markdown_summary(markdown: &str) -> String { + let (mut s, was_shortened) = markdown_summary_with_limit(markdown, Some(59)); + + if was_shortened { + s.push('…'); + } + + s +} + /// Renders the first paragraph of the provided markdown as plain text. +/// Useful for alt-text. /// /// - Headings, links, and formatting are stripped. /// - Inline code is rendered as-is, surrounded by backticks. diff --git a/src/librustdoc/html/markdown/tests.rs b/src/librustdoc/html/markdown/tests.rs index 8e618733f07..9807d8632c7 100644 --- a/src/librustdoc/html/markdown/tests.rs +++ b/src/librustdoc/html/markdown/tests.rs @@ -1,4 +1,4 @@ -use super::plain_text_summary; +use super::{plain_text_summary, short_markdown_summary}; use super::{ErrorCodes, IdMap, Ignore, LangString, Markdown, MarkdownHtml}; use rustc_span::edition::{Edition, DEFAULT_EDITION}; use std::cell::RefCell; @@ -204,6 +204,33 @@ fn test_header_ids_multiple_blocks() { ); } +#[test] +fn test_short_markdown_summary() { + fn t(input: &str, expect: &str) { + let output = short_markdown_summary(input); + assert_eq!(output, expect, "original: {}", input); + } + + t("hello [Rust](https://www.rust-lang.org) :)", "hello Rust :)"); + t("*italic*", "italic"); + 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("
hello
", ""); + t( + "a *very*, **very** long first paragraph. it has lots of `inline code: Vec`. and it has a [link](https://www.rust-lang.org).\nthat was a soft line break! \nthat was a hard one\n\nsecond paragraph.", + "a very, very long first paragraph. it has lots of …", + ); +} + #[test] fn test_plain_text_summary() { fn t(input: &str, expect: &str) { @@ -224,6 +251,10 @@ fn test_plain_text_summary() { t("first paragraph\n\nsecond paragraph", "first paragraph"); t("```\nfn main() {}\n```", ""); t("
hello
", ""); + t( + "a *very*, **very** long first paragraph. it has lots of `inline code: Vec`. and it has a [link](https://www.rust-lang.org).\nthat was a soft line break! \nthat was a hard one\n\nsecond paragraph.", + "a very, very long first paragraph. it has lots of `inline code: Vec`. and it has a link. that was a soft line break! that was a hard one", + ); } #[test] diff --git a/src/librustdoc/html/render/cache.rs b/src/librustdoc/html/render/cache.rs index 085ca01f58d..97f764517fa 100644 --- a/src/librustdoc/html/render/cache.rs +++ b/src/librustdoc/html/render/cache.rs @@ -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_text_summary, shorten}; +use crate::html::markdown::short_markdown_summary; use crate::html::render::{Generic, IndexItem, IndexItemFunctionType, RenderType, TypeWithKind}; /// Indicates where an external crate can be found. @@ -78,7 +78,7 @@ crate 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_text_summary(item.doc_value())), + desc: item.doc_value().map_or_else(|| String::new(), short_markdown_summary), parent: Some(did), parent_idx: None, search_type: get_index_search_type(&item), @@ -127,7 +127,7 @@ crate fn build_index(krate: &clean::Crate, cache: &mut Cache) -> String { let crate_doc = krate .module .as_ref() - .map(|module| shorten(plain_text_summary(module.doc_value()))) + .map(|module| module.doc_value().map_or_else(|| String::new(), short_markdown_summary)) .unwrap_or_default(); #[derive(Serialize)] diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index de620a35c80..901f00b21da 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -76,7 +76,9 @@ use crate::html::format::fmt_impl_for_trait_page; use crate::html::format::Function; use crate::html::format::{href, print_default_space, print_generic_bounds, WhereClause}; use crate::html::format::{print_abi_with_space, Buffer, PrintWithSpace}; -use crate::html::markdown::{self, ErrorCodes, IdMap, Markdown, MarkdownHtml, MarkdownSummaryLine}; +use crate::html::markdown::{ + self, plain_text_summary, ErrorCodes, IdMap, Markdown, MarkdownHtml, MarkdownSummaryLine, +}; use crate::html::sources; use crate::html::{highlight, layout, static_files}; use cache::{build_index, ExternalLocation}; @@ -1604,9 +1606,10 @@ impl Context { Some(ref s) => s.to_string(), }; let short = short.to_string(); - map.entry(short) - .or_default() - .push((myname, Some(plain_text_summary(item.doc_value())))); + map.entry(short).or_default().push(( + myname, + Some(item.doc_value().map_or_else(|| String::new(), plain_text_summary)), + )); } if self.shared.sort_modules_alphabetically { @@ -1810,36 +1813,6 @@ 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_text_summary(s: Option<&str>) -> String { - s.map(markdown::plain_text_summary).unwrap_or_default() -} - -crate fn shorten(s: String) -> String { - if s.chars().count() > 60 { - let mut len = 0; - let mut ret = s - .split_whitespace() - .take_while(|p| { - // + 1 for the added character after the word. - len += p.chars().count() + 1; - len < 60 - }) - .collect::>() - .join(" "); - ret.push('…'); - ret - } else { - s - } -} - fn document(w: &mut Buffer, cx: &Context, item: &clean::Item, parent: Option<&clean::Item>) { if let Some(ref name) = item.name { info!("Documenting {}", name); diff --git a/src/librustdoc/html/static/main.js b/src/librustdoc/html/static/main.js index 69984be5eb6..712ea044e48 100644 --- a/src/librustdoc/html/static/main.js +++ b/src/librustdoc/html/static/main.js @@ -1611,7 +1611,7 @@ function defocusSearchBar() { item.displayPath + "" + name + "" + "" + - "" + escape(item.desc) + + "" + item.desc + " "; }); output += ""; @@ -2013,7 +2013,9 @@ function defocusSearchBar() { } var link = document.createElement("a"); link.href = rootPath + crates[i] + "/index.html"; - link.title = rawSearchIndex[crates[i]].doc; + // The summary in the search index has HTML, so we need to + // dynamically render it as plaintext. + link.title = convertHTMLToPlaintext(rawSearchIndex[crates[i]].doc); link.className = klass; link.textContent = crates[i]; @@ -2026,6 +2028,25 @@ function defocusSearchBar() { } }; + /** + * Convert HTML to plaintext: + * + * * Replace "foo" with "`foo`" + * * Strip all other HTML tags + * + * Used by the dynamic sidebar crate list renderer. + * + * @param {[string]} html [The HTML to convert] + * @return {[string]} [The resulting plaintext] + */ + function convertHTMLToPlaintext(html) { + var dom = new DOMParser().parseFromString( + html.replace('', '`').replace('', '`'), + 'text/html', + ); + return dom.body.innerText; + } + // delayed sidebar rendering. window.initSidebarItems = function(items) { diff --git a/src/test/rustdoc/plain-text-summaries.rs b/src/test/rustdoc/markdown-summaries.rs similarity index 55% rename from src/test/rustdoc/plain-text-summaries.rs rename to src/test/rustdoc/markdown-summaries.rs index c995ccbf0af..b843e28e7b0 100644 --- a/src/test/rustdoc/plain-text-summaries.rs +++ b/src/test/rustdoc/markdown-summaries.rs @@ -1,21 +1,22 @@ #![crate_type = "lib"] #![crate_name = "summaries"] -//! This summary has a [link] and `code`. +//! 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 search-index.js 'This summary has a link and code.' // @!has - 'second paragraph' -/// This `code` should be in backticks. +/// This `code` will be rendered in a code tag. /// /// This text should not be rendered. pub struct Sidebar; -// @has summaries/sidebar-items.js 'This `code` should be in backticks.' +// @has search-index.js 'This code will be rendered in a code tag.' +// @has summaries/sidebar-items.js 'This `code` will be rendered in a code tag.' // @!has - 'text should not be rendered' /// ```text From e178030ea4d1b3a1e38cc53141188d2249d33cf5 Mon Sep 17 00:00:00 2001 From: Camelid Date: Tue, 17 Nov 2020 12:24:16 -0800 Subject: [PATCH 2/6] Use `createElement` and `innerHTML` instead of `DOMParser` @GuillaumeGomez was concerned about browser compatibility. --- src/librustdoc/html/static/main.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/html/static/main.js b/src/librustdoc/html/static/main.js index 712ea044e48..0884351a9fd 100644 --- a/src/librustdoc/html/static/main.js +++ b/src/librustdoc/html/static/main.js @@ -2040,11 +2040,9 @@ function defocusSearchBar() { * @return {[string]} [The resulting plaintext] */ function convertHTMLToPlaintext(html) { - var dom = new DOMParser().parseFromString( - html.replace('', '`').replace('', '`'), - 'text/html', - ); - return dom.body.innerText; + var x = document.createElement("div"); + x.innerHTML = html.replace('', '`').replace('', '`'); + return x.innerText; } From 07e9426efba69e8b662f2a896326b157f5d0ba2d Mon Sep 17 00:00:00 2001 From: Camelid Date: Mon, 23 Nov 2020 19:26:15 -0800 Subject: [PATCH 3/6] Make `length_limit` a `usize` --- src/librustdoc/html/markdown.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 6bba5036191..0e4c5410abe 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1043,13 +1043,11 @@ impl MarkdownSummaryLine<'_> { /// /// Returns a tuple of the rendered HTML string and whether the output was shortened /// due to the provided `length_limit`. -fn markdown_summary_with_limit(md: &str, length_limit: Option) -> (String, bool) { +fn markdown_summary_with_limit(md: &str, length_limit: usize) -> (String, bool) { if md.is_empty() { return (String::new(), false); } - let length_limit = length_limit.unwrap_or(u16::MAX) as usize; - let mut s = String::with_capacity(md.len() * 3 / 2); let mut text_length = 0; let mut stopped_early = false; @@ -1115,7 +1113,7 @@ fn markdown_summary_with_limit(md: &str, length_limit: Option) -> (String, /// /// See [`markdown_summary_with_limit`] for details about what is rendered and what is not. crate fn short_markdown_summary(markdown: &str) -> String { - let (mut s, was_shortened) = markdown_summary_with_limit(markdown, Some(59)); + let (mut s, was_shortened) = markdown_summary_with_limit(markdown, 59); if was_shortened { s.push('…'); From b9035194c0c434d89ec776176a88406cc4af66cc Mon Sep 17 00:00:00 2001 From: Camelid Date: Mon, 23 Nov 2020 20:33:41 -0800 Subject: [PATCH 4/6] Add rustdoc-js test Finally! --- src/test/rustdoc-js/basic.rs | 2 +- src/test/rustdoc-js/summaries.js | 7 +++++++ src/test/rustdoc-js/summaries.rs | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 src/test/rustdoc-js/summaries.js create mode 100644 src/test/rustdoc-js/summaries.rs diff --git a/src/test/rustdoc-js/basic.rs b/src/test/rustdoc-js/basic.rs index 1b4963fcebe..da946a58b1c 100644 --- a/src/test/rustdoc-js/basic.rs +++ b/src/test/rustdoc-js/basic.rs @@ -1,2 +1,2 @@ -/// Foo +/// Docs for Foo pub struct Foo; diff --git a/src/test/rustdoc-js/summaries.js b/src/test/rustdoc-js/summaries.js new file mode 100644 index 00000000000..773357610d7 --- /dev/null +++ b/src/test/rustdoc-js/summaries.js @@ -0,0 +1,7 @@ +const QUERY = 'summaries'; + +const EXPECTED = { + 'others': [ + { 'path': '', 'name': 'summaries', 'desc': 'This summary has a link and code.' }, + ], +}; diff --git a/src/test/rustdoc-js/summaries.rs b/src/test/rustdoc-js/summaries.rs new file mode 100644 index 00000000000..beb91e286b6 --- /dev/null +++ b/src/test/rustdoc-js/summaries.rs @@ -0,0 +1,18 @@ +#![crate_type = "lib"] +#![crate_name = "summaries"] + +//! This *summary* has a [link] and `code`. +//! +//! This is the second paragraph. +//! +//! [link]: https://example.com + +/// This `code` will be rendered in a code tag. +/// +/// This text should not be rendered. +pub struct Sidebar; + +/// ```text +/// this block should not be rendered +/// ``` +pub struct Sidebar2; From f0cf5a974ec0730a512fa241dacf6e2c5659625f Mon Sep 17 00:00:00 2001 From: Camelid Date: Tue, 24 Nov 2020 11:18:45 -0800 Subject: [PATCH 5/6] Add more rustdoc-js test cases --- src/test/rustdoc-js/summaries.js | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/test/rustdoc-js/summaries.js b/src/test/rustdoc-js/summaries.js index 773357610d7..f175e47342d 100644 --- a/src/test/rustdoc-js/summaries.js +++ b/src/test/rustdoc-js/summaries.js @@ -1,7 +1,21 @@ -const QUERY = 'summaries'; +// ignore-tidy-linelength -const EXPECTED = { - 'others': [ - { 'path': '', 'name': 'summaries', 'desc': 'This summary has a link and code.' }, - ], -}; +const QUERY = ['summaries', 'summaries::Sidebar', 'summaries::Sidebar2']; + +const EXPECTED = [ + { + 'others': [ + { 'path': '', 'name': 'summaries', 'desc': 'This summary has a link and code.' }, + ], + }, + { + 'others': [ + { 'path': 'summaries', 'name': 'Sidebar', 'desc': 'This code will be rendered in a code tag.' }, + ], + }, + { + 'others': [ + { 'path': 'summaries', 'name': 'Sidebar2', 'desc': '' }, + ], + }, +]; From 376507f47b5f16c0a9c210005de2bcdb270d8920 Mon Sep 17 00:00:00 2001 From: Camelid Date: Thu, 3 Dec 2020 13:08:40 -0800 Subject: [PATCH 6/6] Add missing feature flag Accidentally removed in rebase. --- src/librustdoc/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 80a9c3811cf..26bf4b569ff 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -15,6 +15,7 @@ #![feature(never_type)] #![feature(once_cell)] #![feature(type_ascription)] +#![feature(split_inclusive)] #![recursion_limit = "256"] #[macro_use]