Prevent spans crossing line boundaries in class-based code block formatter (#2237)

* Prevent spans crossing line boundaries in class formatter

* Add snapshot tests for classed highlighting

Author: TheOnlyMrCat

components/markdown/src/codeblock/highlight.rs

@@ -6,7 +6,9 @@ use libs::syntect::highlighting::{Color, Theme};
 use libs::syntect::html::{
     line_tokens_to_classed_spans, styled_line_to_highlighted_html, ClassStyle, IncludeBackground,
 };
-use libs::syntect::parsing::{ParseState, ScopeStack, SyntaxReference, SyntaxSet};
+use libs::syntect::parsing::{
+    ParseState, Scope, ScopeStack, SyntaxReference, SyntaxSet, SCOPE_REPO,
+};
 use libs::tera::escape_html;
 
 /// Not public, but from syntect::html
@@ -18,17 +20,36 @@ fn write_css_color(s: &mut String, c: Color) {
     }
 }
 
+/// Not public, but from syntect::html
+fn scope_to_classes(s: &mut String, scope: Scope, style: ClassStyle) {
+    let repo = SCOPE_REPO.lock().unwrap();
+    for i in 0..(scope.len()) {
+        let atom = scope.atom_at(i as usize);
+        let atom_s = repo.atom_str(atom);
+        if i != 0 {
+            s.push(' ')
+        }
+        match style {
+            ClassStyle::Spaced => {}
+            ClassStyle::SpacedPrefixed { prefix } => {
+                s.push_str(prefix);
+            }
+            _ => {} // Non-exhaustive
+        }
+        s.push_str(atom_s);
+    }
+}
+
 pub(crate) struct ClassHighlighter<'config> {
     syntax_set: &'config SyntaxSet,
-    open_spans: isize,
     parse_state: ParseState,
     scope_stack: ScopeStack,
 }
 
 impl<'config> ClassHighlighter<'config> {
     pub fn new(syntax: &SyntaxReference, syntax_set: &'config SyntaxSet) -> Self {
         let parse_state = ParseState::new(syntax);
-        Self { syntax_set, open_spans: 0, parse_state, scope_stack: ScopeStack::new() }
+        Self { syntax_set, parse_state, scope_stack: ScopeStack::new() }
     }
 
     /// Parse the line of code and update the internal HTML buffer with tagged HTML
@@ -39,24 +60,28 @@ impl<'config> ClassHighlighter<'config> {
         debug_assert!(line.ends_with('\n'));
         let parsed_line =
             self.parse_state.parse_line(line, self.syntax_set).expect("failed to parse line");
-        let (formatted_line, delta) = line_tokens_to_classed_spans(
+
+        let mut formatted_line = String::with_capacity(line.len() + self.scope_stack.len() * 8); // A guess
+        for scope in self.scope_stack.as_slice() {
+            formatted_line.push_str("<span class=\"");
+            scope_to_classes(&mut formatted_line, *scope, CLASS_STYLE);
+            formatted_line.push_str("\">");
+        }
+
+        let (formatted_contents, _) = line_tokens_to_classed_spans(
             line,
             parsed_line.as_slice(),
             CLASS_STYLE,
             &mut self.scope_stack,
         )
         .expect("line_tokens_to_classed_spans should not fail");
-        self.open_spans += delta;
-        formatted_line
-    }
+        formatted_line.push_str(&formatted_contents);
 
-    /// Close all open `<span>` tags and return the finished HTML string
-    pub fn finalize(&mut self) -> String {
-        let mut html = String::with_capacity((self.open_spans * 7) as usize);
-        for _ in 0..self.open_spans {
-            html.push_str("</span>");
+        for _ in 0..self.scope_stack.len() {
+            formatted_line.push_str("</span>");
         }
-        html
+
+        formatted_line
     }
 }
 
@@ -130,15 +155,6 @@ impl<'config> SyntaxHighlighter<'config> {
         }
     }
 
-    pub fn finalize(&mut self) -> Option<String> {
-        use SyntaxHighlighter::*;
-
-        match self {
-            Inlined(_) | NoHighlight => None,
-            Classed(h) => Some(h.finalize()),
-        }
-    }
-
     /// Inlined needs to set the background/foreground colour on <pre>
     pub fn pre_style(&self) -> Option<String> {
         use SyntaxHighlighter::*;
@@ -210,7 +226,6 @@ mod tests {
         for line in LinesWithEndings::from(code) {
             out.push_str(&highlighter.highlight_line(line));
         }
-        out.push_str(&highlighter.finalize());
 
         assert!(out.starts_with("<span class"));
         assert!(out.ends_with("</span>"));

components/markdown/src/codeblock/mod.rs

@@ -168,10 +168,6 @@ impl<'config> CodeBlock<'config> {
             }
         }
 
-        if let Some(rest) = self.highlighter.finalize() {
-            buffer.push_str(&rest);
-        }
-
         if self.line_numbers {
             buffer.push_str("</tbody></table>");
         }

components/markdown/tests/codeblocks.rs

@@ -2,9 +2,24 @@ use config::Config;
 
 mod common;
 
-fn render_codeblock(content: &str, highlight_code: bool) -> String {
+enum HighlightMode {
+    None,
+    Inlined,
+    Classed,
+}
+
+fn render_codeblock(content: &str, highlight_mode: HighlightMode) -> String {
     let mut config = Config::default_for_test();
-    config.markdown.highlight_code = highlight_code;
+    match highlight_mode {
+        HighlightMode::None => {}
+        HighlightMode::Inlined => {
+            config.markdown.highlight_code = true;
+        }
+        HighlightMode::Classed => {
+            config.markdown.highlight_code = true;
+            config.markdown.highlight_theme = "css".to_owned();
+        }
+    }
     common::render_with_config(content, config).unwrap().body
 }
 
@@ -17,7 +32,7 @@ foo
 bar
 ```
     "#,
-        false,
+        HighlightMode::None,
     );
     insta::assert_snapshot!(body);
 }
@@ -33,7 +48,7 @@ baz
 bat
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -49,7 +64,7 @@ bar
 baz
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -65,7 +80,7 @@ bar
 baz
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -81,7 +96,7 @@ bar
 baz
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -97,7 +112,7 @@ bar
 baz
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     let body2 = render_codeblock(
         r#"
@@ -108,7 +123,7 @@ bar
 baz
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     assert_eq!(body, body2);
 }
@@ -124,7 +139,7 @@ bar
 baz
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -140,7 +155,7 @@ bar
 baz
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -156,7 +171,7 @@ bar
 baz
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -172,7 +187,7 @@ bar
 baz
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -188,7 +203,7 @@ bar
 baz
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -204,7 +219,7 @@ bar
 baz
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -220,7 +235,24 @@ bar
 baz
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
+    );
+    insta::assert_snapshot!(body);
+}
+
+#[test]
+fn can_highlight_with_classes() {
+    let body = render_codeblock(
+        r#"
+```html,hl_lines=3-4
+<link
+    rel="stylesheet"
+    type="text/css"
+    href="main.css"
+/>
+```
+    "#,
+        HighlightMode::Classed,
     );
     insta::assert_snapshot!(body);
 }
@@ -234,14 +266,14 @@ foo
 bar
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
 
 #[test]
 fn can_add_line_numbers_windows_eol() {
-    let body = render_codeblock("```linenos\r\nfoo\r\nbar\r\n```\r\n", true);
+    let body = render_codeblock("```linenos\r\nfoo\r\nbar\r\n```\r\n", HighlightMode::Inlined);
     insta::assert_snapshot!(body);
 }
 
@@ -254,7 +286,7 @@ foo
 bar
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -268,7 +300,24 @@ foo
 bar
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
+    );
+    insta::assert_snapshot!(body);
+}
+
+#[test]
+fn can_add_line_numbers_with_classes() {
+    let body = render_codeblock(
+        r#"
+```html,linenos
+<link
+    rel="stylesheet"
+    type="text/css"
+    href="main.css"
+/>
+```
+    "#,
+        HighlightMode::Classed,
     );
     insta::assert_snapshot!(body);
 }
@@ -283,7 +332,7 @@ fn can_render_shortcode_in_codeblock() {
 </div>
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -300,7 +349,7 @@ text2
 text3
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -323,7 +372,7 @@ A quote
 <!-- end text goes here -->
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }
@@ -337,7 +386,7 @@ foo
 bar
 ```
     "#,
-        true,
+        HighlightMode::Inlined,
     );
     insta::assert_snapshot!(body);
 }

components/markdown/tests/snapshots/codeblocks__can_add_line_numbers_with_classes.snap

@@ -0,0 +1,11 @@
+---
+source: components/markdown/tests/codeblocks.rs
+expression: body
+---
+<pre data-linenos data-lang="html" class="language-html z-code"><code class="language-html" data-lang="html"><table><tbody><tr><td>1</td><td><span class="z-text z-html z-basic"><span class="z-meta z-tag z-inline z-any z-html"><span class="z-punctuation z-definition z-tag z-begin z-html">&lt;</span><span class="z-entity z-name z-tag z-inline z-any z-html">link</span>
+</span></span></td></tr><tr><td>2</td><td><span class="z-text z-html z-basic"><span class="z-meta z-tag z-inline z-any z-html">    <span class="z-meta z-attribute-with-value z-html"><span class="z-entity z-other z-attribute-name z-html">rel</span><span class="z-punctuation z-separator z-key-value z-html">=</span><span class="z-string z-quoted z-double z-html"><span class="z-punctuation z-definition z-string z-begin z-html">&quot;</span>stylesheet<span class="z-punctuation z-definition z-string z-end z-html">&quot;</span></span></span>
+</span></span></td></tr><tr><td>3</td><td><span class="z-text z-html z-basic"><span class="z-meta z-tag z-inline z-any z-html">    <span class="z-meta z-attribute-with-value z-html"><span class="z-entity z-other z-attribute-name z-html">type</span><span class="z-punctuation z-separator z-key-value z-html">=</span><span class="z-string z-quoted z-double z-html"><span class="z-punctuation z-definition z-string z-begin z-html">&quot;</span>text/css<span class="z-punctuation z-definition z-string z-end z-html">&quot;</span></span></span>
+</span></span></td></tr><tr><td>4</td><td><span class="z-text z-html z-basic"><span class="z-meta z-tag z-inline z-any z-html">    <span class="z-meta z-attribute-with-value z-html"><span class="z-entity z-other z-attribute-name z-html">href</span><span class="z-punctuation z-separator z-key-value z-html">=</span><span class="z-string z-quoted z-double z-html"><span class="z-punctuation z-definition z-string z-begin z-html">&quot;</span>main.css<span class="z-punctuation z-definition z-string z-end z-html">&quot;</span></span></span>
+</span></span></td></tr><tr><td>5</td><td><span class="z-text z-html z-basic"><span class="z-meta z-tag z-inline z-any z-html"><span class="z-punctuation z-definition z-tag z-end z-html">/&gt;</span></span>
+</span></td></tr></tbody></table></code></pre>
+