From 68f327bcc112ffb4261980c04e58d333b5d97b6e Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 11 Aug 2022 23:03:45 +0200 Subject: [PATCH 1/3] Merge HTML elements in highlighting when they can be merged together --- src/librustdoc/html/highlight.rs | 163 +++++++++++++++++++++++++++++-- 1 file changed, 155 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 27ccff9a2768f..517f441e64798 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -111,6 +111,69 @@ fn write_header(out: &mut Buffer, class: &str, extra_content: Option) { write!(out, ""); } +/// Write all the pending elements sharing a same (or at mergeable) `Class`. +/// +/// If there is a "parent" (if a `EnterSpan` event was encountered) and the parent can be merged +/// with the elements' class, then we simply write the elements since the `ExitSpan` event will +/// close the tag. +/// +/// Otherwise, if there is only one pending element, we let the `string` function handle both +/// opening and closing the tag, otherwise we do it into this function. +fn write_pending_elems( + out: &mut Buffer, + href_context: &Option>, + pending_elems: &mut Vec<(&str, Option)>, + current_class: &mut Option, + closing_tags: &[(&str, Class)], +) { + if pending_elems.is_empty() { + return; + } + let mut done = false; + if let Some((_, parent_class)) = closing_tags.last() { + if can_merge(*current_class, Some(*parent_class), "") { + for (text, class) in pending_elems.iter() { + string(out, Escape(text), *class, &href_context, false); + } + done = true; + } + } + if !done { + // We only want to "open" the tag ourselves if we have more than one pending and if the current + // parent tag is not the same as our pending content. + let open_tag_ourselves = pending_elems.len() > 1; + let close_tag = if open_tag_ourselves { + enter_span(out, current_class.unwrap(), &href_context) + } else { + "" + }; + for (text, class) in pending_elems.iter() { + string(out, Escape(text), *class, &href_context, !open_tag_ourselves); + } + if open_tag_ourselves { + exit_span(out, close_tag); + } + } + pending_elems.clear(); + *current_class = None; +} + +/// Check if two `Class` can be merged together. In the following rules, "unclassified" means `None` +/// basically (since it's `Option`). The following rules apply: +/// +/// * If two `Class` have the same variant, then they can be merged. +/// * If the other `Class` is unclassified and only contains white characters (backline, +/// whitespace, etc), it can be merged. +/// * If `Class` is `Ident`, then it can be merged with all unclassified elements. +fn can_merge(class1: Option, class2: Option, text: &str) -> bool { + match (class1, class2) { + (Some(c1), Some(c2)) => c1.is_equal_to(c2), + (Some(Class::Ident(_)), None) | (None, Some(Class::Ident(_))) => true, + (Some(_), None) | (None, Some(_)) => text.trim().is_empty(), + _ => false, + } +} + /// Convert the given `src` source code into HTML by adding classes for highlighting. /// /// This code is used to render code blocks (in the documentation) as well as the source code pages. @@ -130,7 +193,15 @@ fn write_code( ) { // This replace allows to fix how the code source with DOS backline characters is displayed. let src = src.replace("\r\n", "\n"); - let mut closing_tags: Vec<&'static str> = Vec::new(); + // It contains the closing tag and the associated `Class`. + let mut closing_tags: Vec<(&'static str, Class)> = Vec::new(); + // The following two variables are used to group HTML elements with same `class` attributes + // to reduce the DOM size. + let mut current_class: Option = None; + // We need to keep the `Class` for each element because it could contain a `Span` which is + // used to generate links. + let mut pending_elems: Vec<(&str, Option)> = Vec::new(); + Classifier::new( &src, href_context.as_ref().map(|c| c.file_span).unwrap_or(DUMMY_SP), @@ -138,15 +209,48 @@ fn write_code( ) .highlight(&mut |highlight| { match highlight { - Highlight::Token { text, class } => string(out, Escape(text), class, &href_context), + Highlight::Token { text, class } => { + // If the two `Class` are different, time to flush the current content and start + // a new one. + if !can_merge(current_class, class, text) { + write_pending_elems( + out, + &href_context, + &mut pending_elems, + &mut current_class, + &closing_tags, + ); + current_class = class.map(Class::dummy); + } else if current_class.is_none() { + current_class = class.map(Class::dummy); + } + pending_elems.push((text, class)); + } Highlight::EnterSpan { class } => { - closing_tags.push(enter_span(out, class, &href_context)) + // We flush everything just in case... + write_pending_elems( + out, + &href_context, + &mut pending_elems, + &mut current_class, + &closing_tags, + ); + closing_tags.push((enter_span(out, class, &href_context), class)) } Highlight::ExitSpan => { - exit_span(out, closing_tags.pop().expect("ExitSpan without EnterSpan")) + // We flush everything just in case... + write_pending_elems( + out, + &href_context, + &mut pending_elems, + &mut current_class, + &closing_tags, + ); + exit_span(out, closing_tags.pop().expect("ExitSpan without EnterSpan").0) } }; }); + write_pending_elems(out, &href_context, &mut pending_elems, &mut current_class, &closing_tags); } fn write_footer(out: &mut Buffer, playground_button: Option<&str>) { @@ -177,6 +281,31 @@ enum Class { } impl Class { + /// It is only looking at the variant, not the variant content. + /// + /// It is used mostly to group multiple similar HTML elements into one `` instead of + /// multiple ones. + fn is_equal_to(self, other: Self) -> bool { + match (self, other) { + (Self::Self_(_), Self::Self_(_)) + | (Self::Macro(_), Self::Macro(_)) + | (Self::Ident(_), Self::Ident(_)) + | (Self::Decoration(_), Self::Decoration(_)) => true, + (x, y) => x == y, + } + } + + /// If `self` contains a `Span`, it'll be replaced with `DUMMY_SP` to prevent creating links + /// on "empty content" (because of the attributes merge). + fn dummy(self) -> Self { + match self { + Self::Self_(_) => Self::Self_(DUMMY_SP), + Self::Macro(_) => Self::Macro(DUMMY_SP), + Self::Ident(_) => Self::Ident(DUMMY_SP), + s => s, + } + } + /// Returns the css class expected by rustdoc for each `Class`. fn as_html(self) -> &'static str { match self { @@ -630,7 +759,7 @@ impl<'a> Classifier<'a> { TokenKind::CloseBracket => { if self.in_attribute { self.in_attribute = false; - sink(Highlight::Token { text: "]", class: None }); + sink(Highlight::Token { text: "]", class: Some(Class::Attribute) }); sink(Highlight::ExitSpan); return; } @@ -701,7 +830,7 @@ fn enter_span( klass: Class, href_context: &Option>, ) -> &'static str { - string_without_closing_tag(out, "", Some(klass), href_context).expect( + string_without_closing_tag(out, "", Some(klass), href_context, true).expect( "internal error: enter_span was called with Some(klass) but did not return a \ closing HTML tag", ) @@ -733,8 +862,10 @@ fn string( text: T, klass: Option, href_context: &Option>, + open_tag: bool, ) { - if let Some(closing_tag) = string_without_closing_tag(out, text, klass, href_context) { + if let Some(closing_tag) = string_without_closing_tag(out, text, klass, href_context, open_tag) + { out.write_str(closing_tag); } } @@ -753,6 +884,7 @@ fn string_without_closing_tag( text: T, klass: Option, href_context: &Option>, + open_tag: bool, ) -> Option<&'static str> { let Some(klass) = klass else { @@ -761,6 +893,10 @@ fn string_without_closing_tag( }; let Some(def_span) = klass.get_span() else { + if !open_tag { + write!(out, "{}", text); + return None; + } write!(out, "{}", klass.as_html(), text); return Some(""); }; @@ -784,6 +920,7 @@ fn string_without_closing_tag( path }); } + // We don't want to generate links on empty text. if let Some(href_context) = href_context { if let Some(href) = href_context.context.shared.span_correspondance_map.get(&def_span).and_then(|href| { @@ -812,10 +949,20 @@ fn string_without_closing_tag( } }) { - write!(out, "{}", klass.as_html(), href, text_s); + if !open_tag { + // We're already inside an element which has the same klass, no need to give it + // again. + write!(out, "{}", href, text_s); + } else { + write!(out, "{}", klass.as_html(), href, text_s); + } return Some(""); } } + if !open_tag { + write!(out, "{}", text_s); + return None; + } write!(out, "{}", klass.as_html(), text_s); Some("") } From 33df8a96553fc99f9792c70a8a28491f0519718c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 12 Aug 2022 18:18:04 +0200 Subject: [PATCH 2/3] Don't generate ident elements as DOM nodes --- src/librustdoc/html/highlight.rs | 27 ++++++++++++++----- src/librustdoc/html/static/css/themes/ayu.css | 6 +---- .../html/static/css/themes/dark.css | 2 +- .../html/static/css/themes/light.css | 2 +- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 517f441e64798..9d8ee52a3faf8 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -164,7 +164,8 @@ fn write_pending_elems( /// * If two `Class` have the same variant, then they can be merged. /// * If the other `Class` is unclassified and only contains white characters (backline, /// whitespace, etc), it can be merged. -/// * If `Class` is `Ident`, then it can be merged with all unclassified elements. +/// * `Class::Ident` is considered the same as unclassified (because it doesn't have an associated +/// CSS class). fn can_merge(class1: Option, class2: Option, text: &str) -> bool { match (class1, class2) { (Some(c1), Some(c2)) => c1.is_equal_to(c2), @@ -264,7 +265,7 @@ enum Class { DocComment, Attribute, KeyWord, - // Keywords that do pointer/reference stuff. + /// Keywords that do pointer/reference stuff. RefKeyWord, Self_(Span), Macro(Span), @@ -272,6 +273,7 @@ enum Class { String, Number, Bool, + /// `Ident` isn't rendered in the HTML but we still need it for the `Span` it contains. Ident(Span), Lifetime, PreludeTy, @@ -320,7 +322,7 @@ impl Class { Class::String => "string", Class::Number => "number", Class::Bool => "bool-val", - Class::Ident(_) => "ident", + Class::Ident(_) => "", Class::Lifetime => "lifetime", Class::PreludeTy => "prelude-ty", Class::PreludeVal => "prelude-val", @@ -920,7 +922,7 @@ fn string_without_closing_tag( path }); } - // We don't want to generate links on empty text. + if let Some(href_context) = href_context { if let Some(href) = href_context.context.shared.span_correspondance_map.get(&def_span).and_then(|href| { @@ -954,7 +956,12 @@ fn string_without_closing_tag( // again. write!(out, "{}", href, text_s); } else { - write!(out, "{}", klass.as_html(), href, text_s); + let klass_s = klass.as_html(); + if klass_s.is_empty() { + write!(out, "{}", href, text_s); + } else { + write!(out, "{}", klass_s, href, text_s); + } } return Some(""); } @@ -963,8 +970,14 @@ fn string_without_closing_tag( write!(out, "{}", text_s); return None; } - write!(out, "{}", klass.as_html(), text_s); - Some("") + let klass_s = klass.as_html(); + if klass_s.is_empty() { + write!(out, "{}", text_s); + Some("") + } else { + write!(out, "{}", klass_s, text_s); + Some("") + } } #[cfg(test)] diff --git a/src/librustdoc/html/static/css/themes/ayu.css b/src/librustdoc/html/static/css/themes/ayu.css index b8218867a8bc8..4dfb64abbebe8 100644 --- a/src/librustdoc/html/static/css/themes/ayu.css +++ b/src/librustdoc/html/static/css/themes/ayu.css @@ -250,9 +250,6 @@ pre.rust .self { pre.rust .attribute { color: #e6e1cf; } -pre.rust .attribute .ident { - color: #e6e1cf; -} .example-wrap > pre.line-number { color: #5c67736e; @@ -398,8 +395,7 @@ pre.rust .comment {} .block a.current.method,.content span.tymethod,.content a.tymethod,.block a.current.tymethod, .content .fnname {} pre.rust .kw {} -pre.rust .self,pre.rust .bool-val,pre.rust .prelude-val,pre.rust .attribute, -pre.rust .attribute .ident {} +pre.rust .self,pre.rust .bool-val,pre.rust .prelude-val,pre.rust .attribute {} .content span.foreigntype,.content a.foreigntype,.block a.current.foreigntype {} pre.rust .doccomment {} .stab.deprecated {} diff --git a/src/librustdoc/html/static/css/themes/dark.css b/src/librustdoc/html/static/css/themes/dark.css index 6188e0ac0a9d3..39f83c9980827 100644 --- a/src/librustdoc/html/static/css/themes/dark.css +++ b/src/librustdoc/html/static/css/themes/dark.css @@ -202,7 +202,7 @@ pre.rust .kw { color: #ab8ac1; } pre.rust .kw-2, pre.rust .prelude-ty { color: #769acb; } pre.rust .number, pre.rust .string { color: #83a300; } pre.rust .self, pre.rust .bool-val, pre.rust .prelude-val, -pre.rust .attribute, pre.rust .attribute .ident { color: #ee6868; } +pre.rust .attribute { color: #ee6868; } pre.rust .macro, pre.rust .macro-nonterminal { color: #3E999F; } pre.rust .lifetime { color: #d97f26; } pre.rust .question-mark { diff --git a/src/librustdoc/html/static/css/themes/light.css b/src/librustdoc/html/static/css/themes/light.css index fba790b619371..5698088c790bb 100644 --- a/src/librustdoc/html/static/css/themes/light.css +++ b/src/librustdoc/html/static/css/themes/light.css @@ -184,7 +184,7 @@ pre.rust .kw { color: #8959A8; } pre.rust .kw-2, pre.rust .prelude-ty { color: #4271AE; } pre.rust .number, pre.rust .string { color: #718C00; } pre.rust .self, pre.rust .bool-val, pre.rust .prelude-val, -pre.rust .attribute, pre.rust .attribute .ident { color: #C82829; } +pre.rust .attribute { color: #C82829; } pre.rust .comment { color: #8E908C; } pre.rust .doccomment { color: #4D4D4C; } pre.rust .macro, pre.rust .macro-nonterminal { color: #3E999F; } From 6e574e100c47d1800e05d14cbaae959c11f320ed Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 11 Aug 2022 23:04:01 +0200 Subject: [PATCH 3/3] Update rustdoc tests --- .../html/highlight/fixtures/decorations.html | 4 +- .../html/highlight/fixtures/dos_line.html | 2 +- .../html/highlight/fixtures/highlight.html | 8 ++-- .../html/highlight/fixtures/sample.html | 38 +++++++++---------- .../html/highlight/fixtures/union.html | 10 ++--- src/test/rustdoc/issue-41783.codeblock.html | 5 +++ src/test/rustdoc/issue-41783.rs | 8 ++-- src/test/rustdoc/macro_rules-matchers.rs | 6 +-- 8 files changed, 41 insertions(+), 40 deletions(-) create mode 100644 src/test/rustdoc/issue-41783.codeblock.html diff --git a/src/librustdoc/html/highlight/fixtures/decorations.html b/src/librustdoc/html/highlight/fixtures/decorations.html index ae4dba116d637..2184897872153 100644 --- a/src/librustdoc/html/highlight/fixtures/decorations.html +++ b/src/librustdoc/html/highlight/fixtures/decorations.html @@ -1,2 +1,2 @@ -let x = 1; -let y = 2; \ No newline at end of file +let x = 1; +let y = 2; \ No newline at end of file diff --git a/src/librustdoc/html/highlight/fixtures/dos_line.html b/src/librustdoc/html/highlight/fixtures/dos_line.html index 1c8dbffe78c22..30b50ca7c662c 100644 --- a/src/librustdoc/html/highlight/fixtures/dos_line.html +++ b/src/librustdoc/html/highlight/fixtures/dos_line.html @@ -1,3 +1,3 @@ -pub fn foo() { +pub fn foo() { println!("foo"); } diff --git a/src/librustdoc/html/highlight/fixtures/highlight.html b/src/librustdoc/html/highlight/fixtures/highlight.html index 17f23278ec1f2..9f73e03f95e41 100644 --- a/src/librustdoc/html/highlight/fixtures/highlight.html +++ b/src/librustdoc/html/highlight/fixtures/highlight.html @@ -1,4 +1,4 @@ -use crate::a::foo; -use self::whatever; -let x = super::b::foo; -let y = Self::whatever; \ No newline at end of file +use crate::a::foo; +use self::whatever; +let x = super::b::foo; +let y = Self::whatever; \ No newline at end of file diff --git a/src/librustdoc/html/highlight/fixtures/sample.html b/src/librustdoc/html/highlight/fixtures/sample.html index ea797fd99d3f4..ae2650528eb72 100644 --- a/src/librustdoc/html/highlight/fixtures/sample.html +++ b/src/librustdoc/html/highlight/fixtures/sample.html @@ -8,30 +8,30 @@ .lifetime { color: #B76514; } .question-mark { color: #ff9011; } -
#![crate_type = "lib"]
+
#![crate_type = "lib"]
 
-use std::path::{Path, PathBuf};
+use std::path::{Path, PathBuf};
 
-#[cfg(target_os = "linux")]
-fn main() -> () {
-    let foo = true && false || true;
-    let _: *const () = 0;
-    let _ = &foo;
-    let _ = &&foo;
-    let _ = *foo;
-    mac!(foo, &mut bar);
-    assert!(self.length < N && index <= self.length);
-    ::std::env::var("gateau").is_ok();
-    #[rustfmt::skip]
-    let s:std::path::PathBuf = std::path::PathBuf::new();
-    let mut s = String::new();
+#[cfg(target_os = "linux")]
+fn main() -> () {
+    let foo = true && false || true;
+    let _: *const () = 0;
+    let _ = &foo;
+    let _ = &&foo;
+    let _ = *foo;
+    mac!(foo, &mut bar);
+    assert!(self.length < N && index <= self.length);
+    ::std::env::var("gateau").is_ok();
+    #[rustfmt::skip]
+    let s:std::path::PathBuf = std::path::PathBuf::new();
+    let mut s = String::new();
 
-    match &s {
-        ref mut x => {}
+    match &s {
+        ref mut x => {}
     }
 }
 
-macro_rules! bar {
-    ($foo:tt) => {};
+macro_rules! bar {
+    ($foo:tt) => {};
 }
 

diff --git a/src/librustdoc/html/highlight/fixtures/union.html b/src/librustdoc/html/highlight/fixtures/union.html
index ac8bd28f6c362..9f8915282642d 100644
--- a/src/librustdoc/html/highlight/fixtures/union.html
+++ b/src/librustdoc/html/highlight/fixtures/union.html
@@ -1,8 +1,8 @@
-union Foo {
-    i: i8,
-    u: i8,
+union Foo {
+    i: i8,
+    u: i8,
 }
 
-fn main() {
-    let union = 0;
+fn main() {
+    let union = 0;
 }
diff --git a/src/test/rustdoc/issue-41783.codeblock.html b/src/test/rustdoc/issue-41783.codeblock.html
new file mode 100644
index 0000000000000..b919935e4b47a
--- /dev/null
+++ b/src/test/rustdoc/issue-41783.codeblock.html
@@ -0,0 +1,5 @@
+# single
+## double
+### triple
+#[outer]
+#![inner]
\ No newline at end of file
diff --git a/src/test/rustdoc/issue-41783.rs b/src/test/rustdoc/issue-41783.rs
index 58a55a73815d0..d67716028799b 100644
--- a/src/test/rustdoc/issue-41783.rs
+++ b/src/test/rustdoc/issue-41783.rs
@@ -1,11 +1,9 @@
 // @has issue_41783/struct.Foo.html
 // @!hasraw - 'space'
 // @!hasraw - 'comment'
-// @hasraw - '# single'
-// @hasraw - '## double'
-// @hasraw - '### triple'
-// @hasraw - '#[outer]'
-// @hasraw - '#![inner]'
+// @hasraw - '#[outer]'
+// @hasraw - '#![inner]'
+// @snapshot 'codeblock' - '//*[@class="rustdoc-toggle top-doc"]/*[@class="docblock"]//pre/code'
 
 /// ```no_run
 /// # # space
diff --git a/src/test/rustdoc/macro_rules-matchers.rs b/src/test/rustdoc/macro_rules-matchers.rs
index 98026663e5a7a..96f4126c7c277 100644
--- a/src/test/rustdoc/macro_rules-matchers.rs
+++ b/src/test/rustdoc/macro_rules-matchers.rs
@@ -5,14 +5,12 @@
 
 // @has 'foo/macro.todo.html'
 // @has - '//span[@class="macro"]' 'macro_rules!'
-// @has - '//span[@class="ident"]' 'todo'
+// @hasraw - ' todo {'
 
 // @hasraw - '{ () => { ... }; ($('
 // @has - '//span[@class="macro-nonterminal"]' '$'
 // @has - '//span[@class="macro-nonterminal"]' 'arg'
-// @hasraw - ':'
-// @has - '//span[@class="ident"]' 'tt'
-// @hasraw - ')+'
+// @hasraw - ':tt)+'
 // @hasraw - ') => { ... }; }'
 pub use std::todo;