Repository: rust-lang/annotate-snippets-rs
Pull request #99 (2024-03-12 21:43:09)

fix: Make spans work with single width emojis
In #90, I made the mistake of thinking bytes and character widths were the same thing, and boy, was I very wrong. This PR addresses this problem and ensures that we are properly dealing with characters with different widths from the number of bytes they are made up of.


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 5672f61..3bfb779 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -26,7 +26,6 @@ dependencies = [
  "criterion",
  "difference",
  "glob",
- "itertools 0.12.1",
  "serde",
  "snapbox",
  "toml",
@@ -243,7 +242,7 @@ dependencies = [
  "clap",
  "criterion-plot",
  "is-terminal",
- "itertools 0.10.5",
+ "itertools",
  "num-traits",
  "once_cell",
  "oorandom",
@@ -264,7 +263,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "6b50826342786a51a89e2da3a28f1c32b06e387201bc2d19791f622c673706b1"
 dependencies = [
  "cast",
- "itertools 0.10.5",
+ "itertools",
 ]
 
 [[package]]
@@ -449,15 +448,6 @@ dependencies = [
  "either",
 ]
 
-[[package]]
-name = "itertools"
-version = "0.12.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ba291022dbbd398a455acf126c1e341954079855bc60dfdda641363bd6922569"
-dependencies = [
- "either",
-]
-
 [[package]]
 name = "itoa"
 version = "1.0.9"
diff --git a/Cargo.toml b/Cargo.toml
index 5fbfc0b..f97dddb 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -25,7 +25,6 @@ maintenance = { status = "actively-developed" }
 
 [dependencies]
 anstyle = "1.0.4"
-itertools = "0.12.1"
 unicode-width = "0.1.11"
 
 [dev-dependencies]
diff --git a/src/renderer/display_list.rs b/src/renderer/display_list.rs
index d0444a3..5cc1e2c 100644
--- a/src/renderer/display_list.rs
+++ b/src/renderer/display_list.rs
@@ -32,8 +32,6 @@
 //!
 //! The above snippet has been built out of the following structure:
 use crate::snippet;
-use itertools::FoldWhile::{Continue, Done};
-use itertools::Itertools;
 use std::fmt::{Display, Write};
 use std::ops::Range;
 use std::{cmp, fmt};
@@ -830,20 +828,7 @@ fn format_header<'a>(
             } = item
             {
                 if main_range >= range.0 && main_range <= range.1 {
-                    let char_column = text
-                        .chars()
-                        .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
-                        .chain(std::iter::once(1)) // treat the end of line as single-width
-                        .enumerate()
-                        .fold_while((0, 0), |(count, acc), (i, width)| {
-                            if acc <= main_range - range.0 {
-                                Continue((i, acc + width))
-                            } else {
-                                Done((count, acc))
-                            }
-                        })
-                        .into_inner()
-                        .0;
+                    let char_column = text[0..(main_range - range.0)].chars().count();
                     col = char_column + 1;
                     line_offset = lineno.unwrap_or(1);
                     break;
@@ -984,18 +969,11 @@ fn format_body(
     let mut body = vec![];
     let mut current_line = snippet.line_start;
     let mut current_index = 0;
-    let mut line_info = vec![];
 
-    struct LineInfo {
-        line_start_index: usize,
-        line_end_index: usize,
-    }
-
-    for (line, end_line) in CursorLines::new(snippet.source) {
-        let line_length: usize = line
-            .chars()
-            .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
-            .sum();
+    let mut annotation_line_count = 0;
+    let mut annotations = snippet.annotations;
+    for (idx, (line, end_line)) in CursorLines::new(snippet.source).enumerate() {
+        let line_length: usize = line.len();
         let line_range = (current_index, current_index + line_length);
         body.push(DisplayLine::Source {
             lineno: Some(current_line),
@@ -1005,24 +983,11 @@ fn format_body(
                 range: line_range,
             },
         });
-        line_info.push(LineInfo {
-            line_start_index: line_range.0,
-            line_end_index: line_range.1,
-        });
+        let line_start_index = line_range.0;
+        let line_end_index = line_range.1;
         current_line += 1;
         current_index += line_length + end_line as usize;
-    }
 
-    let mut annotation_line_count = 0;
-    let mut annotations = snippet.annotations;
-    for (
-        idx,
-        LineInfo {
-            line_start_index,
-            line_end_index,
-        },
-    ) in line_info.into_iter().enumerate()
-    {
         let margin_left = margin
             .map(|m| m.left(line_end_index - line_start_index))
             .unwrap_or_default();
@@ -1040,8 +1005,20 @@ fn format_body(
                     if start >= line_start_index && end <= line_end_index
                         || start == line_end_index && end - start <= 1 =>
                 {
-                    let annotation_start_col = start - line_start_index - margin_left;
-                    let annotation_end_col = end - line_start_index - margin_left;
+                    let annotation_start_col = line[0..(start - line_start_index)]
+                        .chars()
+                        .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
+                        .sum::<usize>()
+                        - margin_left;
+                    // This allows for annotations to be placed one past the
+                    // last character
+                    let safe_end = (end - line_start_index).saturating_sub(line_length);
+                    let annotation_end_col = line[0..(end - line_start_index) - safe_end]
+                        .chars()
+                        .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
+                        .sum::<usize>()
+                        + safe_end
+                        - margin_left;
                     let range = (annotation_start_col, annotation_end_col);
                     body.insert(
                         body_idx + 1,
@@ -1080,7 +1057,10 @@ fn format_body(
                             });
                         }
                     } else {
-                        let annotation_start_col = start - line_start_index;
+                        let annotation_start_col = line[0..(start - line_start_index)]
+                            .chars()
+                            .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
+                            .sum::<usize>();
                         let range = (annotation_start_col, annotation_start_col + 1);
                         body.insert(
                             body_idx + 1,
@@ -1132,7 +1112,11 @@ fn format_body(
                         });
                     }
 
-                    let end_mark = (end - line_start_index).saturating_sub(1);
+                    let end_mark = line[0..(end - line_start_index)]
+                        .chars()
+                        .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
+                        .sum::<usize>()
+                        .saturating_sub(1);
                     let range = (end_mark - margin_left, (end_mark + 1) - margin_left);
                     body.insert(
                         body_idx + 1,


Test patch:
diff --git a/tests/fixtures/no-color/ensure-emoji-highlight-width.svg b/tests/fixtures/no-color/ensure-emoji-highlight-width.svg
new file mode 100644
index 0000000..0840805
--- /dev/null
+++ b/tests/fixtures/no-color/ensure-emoji-highlight-width.svg
@@ -0,0 +1,33 @@
+<svg width="1356px" height="128px" xmlns="http://www.w3.org/2000/svg">
+  <style>
+    .fg { fill: #AAAAAA }
+    .bg { background: #000000 }
+    .container {
+      padding: 0 10px;
+      line-height: 18px;
+    }
+    tspan {
+      font: 14px SFMono-Regular, Consolas, Liberation Mono, Menlo, monospace;
+      white-space: pre;
+      line-height: 18px;
+    }
+  </style>
+
+  <rect width="100%" height="100%" y="0" rx="4.5" class="bg" />
+
+  <text xml:space="preserve" class="container fg">
+    <tspan x="10px" y="28px"><tspan>error: invalid character ` ` in package name: `haha this isn't a valid name 🐛`, characters must be Unicode XID characters (numbers, `-`, `_`, or most letters)</tspan>
+</tspan>
+    <tspan x="10px" y="46px"><tspan> --&gt; &lt;file&gt;:7:1</tspan>
+</tspan>
+    <tspan x="10px" y="64px"><tspan>  |</tspan>
+</tspan>
+    <tspan x="10px" y="82px"><tspan>7 | "haha this isn't a valid name 🐛" = { package = "libc", version = "0.1" }</tspan>
+</tspan>
+    <tspan x="10px" y="100px"><tspan>  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^</tspan>
+</tspan>
+    <tspan x="10px" y="118px"><tspan>  |</tspan>
+</tspan>
+  </text>
+
+</svg>
diff --git a/tests/fixtures/no-color/ensure-emoji-highlight-width.toml b/tests/fixtures/no-color/ensure-emoji-highlight-width.toml
new file mode 100644
index 0000000..7af05ff
--- /dev/null
+++ b/tests/fixtures/no-color/ensure-emoji-highlight-width.toml
@@ -0,0 +1,15 @@
+[message]
+title = "invalid character ` ` in package name: `haha this isn't a valid name 🐛`, characters must be Unicode XID characters (numbers, `-`, `_`, or most letters)"
+level = "Error"
+
+
+[[message.snippets]]
+source = """
+"haha this isn't a valid name 🐛" = { package = "libc", version = "0.1" }
+"""
+line_start = 7
+origin = "<file>"
+[[message.snippets.annotations]]
+label = ""
+level = "Error"
+range = [0, 35]
diff --git a/tests/formatter.rs b/tests/formatter.rs
index 76dcb27..035492a 100644
--- a/tests/formatter.rs
+++ b/tests/formatter.rs
@@ -25,7 +25,7 @@ fn test_point_to_double_width_characters() {
     let snippets = Level::Error.title("").snippet(
         Snippet::source("こんにちは、世界")
             .origin("<current file>")
-            .annotation(Level::Error.span(12..16).label("world")),
+            .annotation(Level::Error.span(18..24).label("world")),
     );
 
     let expected = r#"error
@@ -44,7 +44,7 @@ fn test_point_to_double_width_characters_across_lines() {
     let snippets = Level::Error.title("").snippet(
         Snippet::source("おはよう\nございます")
             .origin("<current file>")
-            .annotation(Level::Error.span(4..15).label("Good morning")),
+            .annotation(Level::Error.span(6..22).label("Good morning")),
     );
 
     let expected = r#"error
@@ -65,8 +65,8 @@ fn test_point_to_double_width_characters_multiple() {
     let snippets = Level::Error.title("").snippet(
         Snippet::source("お寿司\n食べたい🍣")
             .origin("<current file>")
-            .annotation(Level::Error.span(0..6).label("Sushi1"))
-            .annotation(Level::Note.span(11..15).label("Sushi2")),
+            .annotation(Level::Error.span(0..9).label("Sushi1"))
+            .annotation(Level::Note.span(16..22).label("Sushi2")),
     );
 
     let expected = r#"error
@@ -87,7 +87,7 @@ fn test_point_to_double_width_characters_mixed() {
     let snippets = Level::Error.title("").snippet(
         Snippet::source("こんにちは、新しいWorld！")
             .origin("<current file>")
-            .annotation(Level::Error.span(12..23).label("New world")),
+            .annotation(Level::Error.span(18..32).label("New world")),
     );
 
     let expected = r#"error