Repository: rust-lang/annotate-snippets-rs
Pull request #90 (2024-03-08 16:20:23)

fix!: Take in byte spans
BREAKING CHANGE: This switches from char spans to byte spans

This PR is a draft implementation to address #77. While I think I fixed every issue in moving from char spans to byte spans, I am not entirely certain. I may look into adding more tests to ensure we are doing the write thing. This also doesn't address the fact that people could pass in char spans when updating the version, not realizing the change. We could do something to make it more apparent but I am not sure what that would be.


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 034e18c..e507736 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -25,6 +25,7 @@ dependencies = [
  "criterion",
  "difference",
  "glob",
+ "itertools 0.12.1",
  "serde",
  "snapbox",
  "toml",
@@ -241,7 +242,7 @@ dependencies = [
  "clap",
  "criterion-plot",
  "is-terminal",
- "itertools",
+ "itertools 0.10.5",
  "num-traits",
  "once_cell",
  "oorandom",
@@ -262,7 +263,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "6b50826342786a51a89e2da3a28f1c32b06e387201bc2d19791f622c673706b1"
 dependencies = [
  "cast",
- "itertools",
+ "itertools 0.10.5",
 ]
 
 [[package]]
@@ -435,6 +436,15 @@ 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 8763c11..0b19a89 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -25,6 +25,7 @@ maintenance = { status = "actively-developed" }
 
 [dependencies]
 anstyle = "1.0.4"
+itertools = "0.12.1"
 unicode-width = "0.1.11"
 
 [dev-dependencies]
diff --git a/benches/simple.rs b/benches/simple.rs
index 3a40bbf..f6abcee 100644
--- a/benches/simple.rs
+++ b/benches/simple.rs
@@ -38,12 +38,12 @@ fn create_snippet(renderer: Renderer) {
                 SourceAnnotation {
                     label: "expected `Option<String>` because of return type",
                     annotation_type: AnnotationType::Warning,
-                    range: (5, 19),
+                    range: 5..19,
                 },
                 SourceAnnotation {
                     label: "expected enum `std::option::Option`",
                     annotation_type: AnnotationType::Error,
-                    range: (26, 724),
+                    range: 26..724,
                 },
             ],
         }],
diff --git a/examples/expected_type.rs b/examples/expected_type.rs
index bbd1fe6..613cf60 100644
--- a/examples/expected_type.rs
+++ b/examples/expected_type.rs
@@ -20,12 +20,12 @@ fn main() {
                 SourceAnnotation {
                     label: "",
                     annotation_type: AnnotationType::Error,
-                    range: (193, 195),
+                    range: 193..195,
                 },
                 SourceAnnotation {
                     label: "while parsing this struct",
                     annotation_type: AnnotationType::Info,
-                    range: (34, 50),
+                    range: 34..50,
                 },
             ],
         }],
diff --git a/examples/footer.rs b/examples/footer.rs
index ca02119..433aa83 100644
--- a/examples/footer.rs
+++ b/examples/footer.rs
@@ -21,7 +21,7 @@ fn main() {
             fold: false,
             annotations: vec![SourceAnnotation {
                 label: "expected struct `annotate_snippets::snippet::Slice`, found reference",
-                range: (21, 24),
+                range: 21..24,
                 annotation_type: AnnotationType::Error,
             }],
         }],
diff --git a/examples/format.rs b/examples/format.rs
index 41f852e..a699f0a 100644
--- a/examples/format.rs
+++ b/examples/format.rs
@@ -32,12 +32,12 @@ fn main() {
                 SourceAnnotation {
                     label: "expected `Option<String>` because of return type",
                     annotation_type: AnnotationType::Warning,
-                    range: (5, 19),
+                    range: 5..19,
                 },
                 SourceAnnotation {
                     label: "expected enum `std::option::Option`",
                     annotation_type: AnnotationType::Error,
-                    range: (26, 724),
+                    range: 26..724,
                 },
             ],
         }],
diff --git a/src/renderer/display_list.rs b/src/renderer/display_list.rs
index f283e52..f8241db 100644
--- a/src/renderer/display_list.rs
+++ b/src/renderer/display_list.rs
@@ -32,7 +32,10 @@
 //!
 //! 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};
 
 use crate::renderer::{stylesheet::Stylesheet, Margin, Style};
@@ -766,7 +769,7 @@ fn format_slice(
     has_footer: bool,
     margin: Option<Margin>,
 ) -> Vec<DisplayLine<'_>> {
-    let main_range = slice.annotations.first().map(|x| x.range.0);
+    let main_range = slice.annotations.first().map(|x| x.range.start);
     let origin = slice.origin;
     let need_empty_header = origin.is_some() || is_first;
     let mut body = format_body(slice, need_empty_header, has_footer, margin);
@@ -804,13 +807,27 @@ fn format_header<'a>(
 
         for item in body {
             if let DisplayLine::Source {
-                line: DisplaySourceLine::Content { range, .. },
+                line: DisplaySourceLine::Content { text, range },
                 lineno,
                 ..
             } = item
             {
                 if main_range >= range.0 && main_range <= range.1 {
-                    col = main_range - range.0 + 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;
+                    col = char_column + 1;
                     line_offset = lineno.unwrap_or(1);
                     break;
                 }
@@ -932,11 +949,11 @@ fn format_body(
     has_footer: bool,
     margin: Option<Margin>,
 ) -> Vec<DisplayLine<'_>> {
-    let source_len = slice.source.chars().count();
+    let source_len = slice.source.len();
     if let Some(bigger) = slice.annotations.iter().find_map(|x| {
         // Allow highlighting one past the last character in the source.
-        if source_len + 1 < x.range.1 {
-            Some(x.range)
+        if source_len + 1 < x.range.end {
+            Some(&x.range)
         } else {
             None
         }
@@ -955,18 +972,14 @@ fn format_body(
     struct LineInfo {
         line_start_index: usize,
         line_end_index: usize,
-        // How many spaces each character in the line take up when displayed
-        char_widths: Vec<usize>,
     }
 
     for (line, end_line) in CursorLines::new(slice.source) {
-        let line_length = line.chars().count();
-        let line_range = (current_index, current_index + line_length);
-        let char_widths = line
+        let line_length: usize = line
             .chars()
             .map(|c| unicode_width::UnicodeWidthChar::width(c).unwrap_or(0))
-            .chain(std::iter::once(1)) // treat the end of line as single-width
-            .collect::<Vec<_>>();
+            .sum();
+        let line_range = (current_index, current_index + line_length);
         body.push(DisplayLine::Source {
             lineno: Some(current_line),
             inline_marks: vec![],
@@ -978,7 +991,6 @@ fn format_body(
         line_info.push(LineInfo {
             line_start_index: line_range.0,
             line_end_index: line_range.1,
-            char_widths,
         });
         current_line += 1;
         current_index += line_length + end_line as usize;
@@ -991,7 +1003,6 @@ fn format_body(
         LineInfo {
             line_start_index,
             line_end_index,
-            char_widths,
         },
     ) in line_info.into_iter().enumerate()
     {
@@ -1007,21 +1018,13 @@ fn format_body(
                 _ => DisplayAnnotationType::from(annotation.annotation_type),
             };
             match annotation.range {
-                (start, _) if start > line_end_index => true,
-                (start, end)
+                Range { start, .. } if start > line_end_index => true,
+                Range { start, end }
                     if start >= line_start_index && end <= line_end_index
                         || start == line_end_index && end - start <= 1 =>
                 {
-                    let annotation_start_col = char_widths
-                        .iter()
-                        .take(start - line_start_index)
-                        .sum::<usize>()
-                        - margin_left;
-                    let annotation_end_col = char_widths
-                        .iter()
-                        .take(end - line_start_index)
-                        .sum::<usize>()
-                        - margin_left;
+                    let annotation_start_col = start - line_start_index - margin_left;
+                    let annotation_end_col = end - line_start_index - margin_left;
                     let range = (annotation_start_col, annotation_end_col);
                     body.insert(
                         body_idx + 1,
@@ -1045,7 +1048,7 @@ fn format_body(
                     annotation_line_count += 1;
                     false
                 }
-                (start, end)
+                Range { start, end }
                     if start >= line_start_index
                         && start <= line_end_index
                         && end > line_end_index =>
@@ -1064,10 +1067,7 @@ fn format_body(
                             });
                         }
                     } else {
-                        let annotation_start_col = char_widths
-                            .iter()
-                            .take(start - line_start_index)
-                            .sum::<usize>();
+                        let annotation_start_col = start - line_start_index;
                         let range = (annotation_start_col, annotation_start_col + 1);
                         body.insert(
                             body_idx + 1,
@@ -1092,7 +1092,7 @@ fn format_body(
                     }
                     true
                 }
-                (start, end) if start < line_start_index && end > line_end_index => {
+                Range { start, end } if start < line_start_index && end > line_end_index => {
                     if let DisplayLine::Source {
                         ref mut inline_marks,
                         ..
@@ -1107,7 +1107,7 @@ fn format_body(
                     }
                     true
                 }
-                (start, end)
+                Range { start, end }
                     if start < line_start_index
                         && end >= line_start_index
                         && end <= line_end_index =>
@@ -1125,11 +1125,7 @@ fn format_body(
                         });
                     }
 
-                    let end_mark = char_widths
-                        .iter()
-                        .take(end - line_start_index)
-                        .sum::<usize>()
-                        .saturating_sub(1);
+                    let end_mark = (end - line_start_index).saturating_sub(1);
                     let range = (end_mark - margin_left, (end_mark + 1) - margin_left);
                     body.insert(
                         body_idx + 1,
@@ -1380,7 +1376,7 @@ mod tests {
         let line_2 = "This is line 2";
         let source = [line_1, line_2].join("\n");
         // In line 2
-        let range = (22, 24);
+        let range = 22..24;
         let input = snippet::Snippet {
             title: None,
             footer: vec![],
@@ -1389,7 +1385,7 @@ mod tests {
                 line_start: 5402,
                 origin: None,
                 annotations: vec![snippet::SourceAnnotation {
-                    range,
+                    range: range.clone(),
                     label: "Test annotation",
                     annotation_type: snippet::AnnotationType::Info,
                 }],
@@ -1430,7 +1426,10 @@ mod tests {
                             style: DisplayTextStyle::Regular,
                         }],
                     },
-                    range: (range.0 - (line_1.len() + 1), range.1 - (line_1.len() + 1)),
+                    range: (
+                        range.start - (line_1.len() + 1),
+                        range.end - (line_1.len() + 1),
+                    ),
                     annotation_type: DisplayAnnotationType::Info,
                     annotation_part: DisplayAnnotationPart::Standalone,
                 },
@@ -1480,7 +1479,7 @@ mod tests {
             footer: vec![],
             slices: vec![snippet::Slice {
                 annotations: vec![snippet::SourceAnnotation {
-                    range: (0, source.len() + 2),
+                    range: 0..source.len() + 2,
                     label,
                     annotation_type: snippet::AnnotationType::Error,
                 }],
@@ -1507,7 +1506,7 @@ mod tests {
                 line_start: 1,
                 origin: Some("<current file>"),
                 annotations: vec![snippet::SourceAnnotation {
-                    range: (19, 23),
+                    range: 19..23,
                     label: "oops",
                     annotation_type: snippet::AnnotationType::Error,
                 }],
diff --git a/src/snippet.rs b/src/snippet.rs
index 02e70cc..7f052f0 100644
--- a/src/snippet.rs
+++ b/src/snippet.rs
@@ -31,6 +31,8 @@
 //! };
 //! ```
 
+use std::ops::Range;
+
 /// Primary structure provided for formatting
 #[derive(Debug, Default)]
 pub struct Snippet<'a> {
@@ -70,7 +72,8 @@ pub enum AnnotationType {
 /// An annotation for a `Slice`.
 #[derive(Debug)]
 pub struct SourceAnnotation<'a> {
-    pub range: (usize, usize),
+    /// The byte range of the annotation in the `source` string
+    pub range: Range<usize>,
     pub label: &'a str,
     pub annotation_type: AnnotationType,
 }


Test patch:
diff --git a/tests/fixtures/deserialize.rs b/tests/fixtures/deserialize.rs
index 1763005..70e06ac 100644
--- a/tests/fixtures/deserialize.rs
+++ b/tests/fixtures/deserialize.rs
@@ -1,4 +1,5 @@
 use serde::{Deserialize, Deserializer, Serialize};
+use std::ops::Range;
 
 use annotate_snippets::{
     renderer::Margin, Annotation, AnnotationType, Renderer, Slice, Snippet, SourceAnnotation,
@@ -122,7 +123,7 @@ where
 #[derive(Serialize, Deserialize)]
 #[serde(remote = "SourceAnnotation")]
 pub struct SourceAnnotationDef<'a> {
-    pub range: (usize, usize),
+    pub range: Range<usize>,
     #[serde(borrow)]
     pub label: &'a str,
     #[serde(with = "AnnotationTypeDef")]
diff --git a/tests/formatter.rs b/tests/formatter.rs
index 97c7be3..954204d 100644
--- a/tests/formatter.rs
+++ b/tests/formatter.rs
@@ -14,7 +14,7 @@ fn test_i_29() {
             line_start: 1,
             origin: Some("<current file>"),
             annotations: vec![SourceAnnotation {
-                range: (19, 23),
+                range: 19..23,
                 label: "oops",
                 annotation_type: AnnotationType::Error,
             }],
@@ -41,7 +41,7 @@ fn test_point_to_double_width_characters() {
             line_start: 1,
             origin: Some("<current file>"),
             annotations: vec![SourceAnnotation {
-                range: (6, 8),
+                range: 12..16,
                 label: "world",
                 annotation_type: AnnotationType::Error,
             }],
@@ -69,7 +69,7 @@ fn test_point_to_double_width_characters_across_lines() {
             line_start: 1,
             origin: Some("<current file>"),
             annotations: vec![SourceAnnotation {
-                range: (2, 8),
+                range: 4..15,
                 label: "Good morning",
                 annotation_type: AnnotationType::Error,
             }],
@@ -100,12 +100,12 @@ fn test_point_to_double_width_characters_multiple() {
             origin: Some("<current file>"),
             annotations: vec![
                 SourceAnnotation {
-                    range: (0, 3),
+                    range: 0..6,
                     label: "Sushi1",
                     annotation_type: AnnotationType::Error,
                 },
                 SourceAnnotation {
-                    range: (6, 8),
+                    range: 11..15,
                     label: "Sushi2",
                     annotation_type: AnnotationType::Note,
                 },
@@ -136,7 +136,7 @@ fn test_point_to_double_width_characters_mixed() {
             line_start: 1,
             origin: Some("<current file>"),
             annotations: vec![SourceAnnotation {
-                range: (6, 14),
+                range: 12..23,
                 label: "New world",
                 annotation_type: AnnotationType::Error,
             }],