Repository: rust-lang/annotate-snippets-rs
Pull request #154 (2024-10-13 22:43:46)

Fix origin position computation
This PR fixes an issue in the computation of the position in the origin file. When a highlighted section starts at the beginning of the line, the origin position used to point to the end of the previous line, instead of the start of the current line. This resulted in outputs like this:

```
error: title
 --> origin.txt:2:4
  |
1 | aaa
2 | bbb
3 | ccc
  | ^^^ annotation
4 | ddd
  |
```

Here `origin.txt:2:4` is wrong, it should be `origin.txt:3:1`, which actually matches the highlighted setting.


As a drive-by fix, 21645ad032dd9117d811db57a5740b6a6406f94a fixes `EndLine`, which was used incorrectly in multiple places that happened to cancel out.


KarelPeeters: Ah, I missed those failing test cases, since they're disabled on windows:
https://github.com/rust-lang/annotate-snippets-rs/blob/c84e388949f0e06f4529de060c195e12e6531fbd/tests/fixtures/main.rs#L9-L14
Is there any reason for that, maybe CRLF issues? They seem to behave as expected for me when I just remove that `#[cfg(not(windows))]` line!

The failing test are about cases that try to point one byte past the end of the line. Previously that was indeed possible, but only in very limited situations, eg. the highlighting only worked on EOF cases anyway. See https://github.com/rust-lang/annotate-snippets-rs/issues/108#issuecomment-2290855597 for vaguely relevant discussion about this.

I'm not sure what the best way to proceed here is. Is the "one past the end of the line" use case already adequately supported by just allowing users to point at the newline characters themselves? Ideally there would be some out-of-band way to signal this case, since only using byte indices for this is tricky. 

Patch:
diff --git a/src/renderer/display_list.rs b/src/renderer/display_list.rs
index e2f0d74..8884b0d 100644
--- a/src/renderer/display_list.rs
+++ b/src/renderer/display_list.rs
@@ -1149,7 +1149,7 @@ fn format_header<'a>(
                 ..
             } = item
             {
-                if main_range >= range.0 && main_range <= range.1 + end_line.len() {
+                if main_range >= range.0 && main_range < range.1 + max(*end_line as usize, 1) {
                     let char_column = text[0..(main_range - range.0).min(text.len())]
                         .chars()
                         .count();


Test patch:
diff --git a/tests/formatter.rs b/tests/formatter.rs
index 7f914de..6faab76 100644
--- a/tests/formatter.rs
+++ b/tests/formatter.rs
@@ -905,3 +905,53 @@ error: unused optional dependency
     let renderer = Renderer::plain();
     assert_data_eq!(renderer.render(input).to_string(), expected);
 }
+
+#[test]
+fn origin_correct_start_line() {
+    let source = "aaa\nbbb\nccc\nddd\n";
+    let input = Level::Error.title("title").snippet(
+        Snippet::source(source)
+            .origin("origin.txt")
+            .fold(false)
+            .annotation(Level::Error.span(8..8 + 3).label("annotation")),
+    );
+
+    let expected = str![[r#"
+error: title
+ --> origin.txt:3:1
+  |
+1 | aaa
+2 | bbb
+3 | ccc
+  | ^^^ annotation
+4 | ddd
+  |
+"#]];
+    let renderer = Renderer::plain();
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}
+
+#[test]
+fn origin_correct_mid_line() {
+    let source = "aaa\nbbb\nccc\nddd\n";
+    let input = Level::Error.title("title").snippet(
+        Snippet::source(source)
+            .origin("origin.txt")
+            .fold(false)
+            .annotation(Level::Error.span(8 + 1..8 + 3).label("annotation")),
+    );
+
+    let expected = str![[r#"
+error: title
+ --> origin.txt:3:2
+  |
+1 | aaa
+2 | bbb
+3 | ccc
+  |  ^^ annotation
+4 | ddd
+  |
+"#]];
+    let renderer = Renderer::plain();
+    assert_data_eq!(renderer.render(input).to_string(), expected);
+}