From b1c4112e9bc360b679564479f985057cef00515f Mon Sep 17 00:00:00 2001
From: Andrew Gallant <jamslam@gmail.com>
Date: Wed, 8 Jan 2025 11:28:39 -0500
Subject: [PATCH] fix: Tweak rendering for single ASCII whitespace
This PR includes a small fix (that I am not at all sure about) and a
regression test that fails on current master.
The input here is just a single ASCII whitespace with an annotation
pointing to immediately after that whitespace. In current master, it
gets rendered like this:
```
error: missing trailing newline at end of file
|
1 | ...
| ^ W292
|
= help: Add trailing newline
```
But I think the insertion of an ellipsis here is not quite right. I was
certainly confused by the output.
I don't really understand the formatting code at all, but I believe the
ellipsis is being inserted by this code:
```rust
if self.margin.was_cut_left() {
// We have stripped some code/whitespace from the beginning, make it clear.
buffer.puts(line_offset, code_offset, "...", *lineno_color);
}
```
And `self.margin.was_cut_left()` was returning true because
`computed_left` was set to `18446744073709551593`. This kind of value
_seems_ like a bug, although some margin values are explicitly
initialized to `usize::MAX`, so maybe not.
Anywho, I tried removing the condition gating the setting of
`whitespace_margin`, and that seems to fix this specific case without
any other _known_ regressions (including across all of `ruff`'s test
suite), but this was mostly a result of me feeling around in the dark
here.
---
src/renderer/display_list.rs | 4 +---
tests/formatter.rs | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/src/renderer/display_list.rs b/src/renderer/display_list.rs
index c2cb82b..1aab628 100644
--- a/src/renderer/display_list.rs
+++ b/src/renderer/display_list.rs
@@ -1400,9 +1400,7 @@ fn format_body(
}
})
.sum();
- if line.chars().any(|c| !c.is_whitespace()) {
- whitespace_margin = min(whitespace_margin, leading_whitespace);
- }
+ whitespace_margin = min(whitespace_margin, leading_whitespace);
max_line_len = max(max_line_len, line_length);
let line_start_index = line_range.0;
diff --git a/tests/formatter.rs b/tests/formatter.rs
index 6faab76..0c5a363 100644
--- a/tests/formatter.rs
+++ b/tests/formatter.rs
@@ -955,3 +955,29 @@ error: title
let renderer = Renderer::plain();
assert_data_eq!(renderer.render(input).to_string(), expected);
}
+
+// This tests that reasonable rendering is done in an odd case: when the source
+// is a single ASCII whitespace and there's annotation pointing to immediately
+// after it.
+//
+// Previously, this would end up with a `...` rendered instead of just the
+// space itself. The `...` seems incorrect here because I don't believe any
+// trimming occurs (or is needed).
+#[test]
+fn one_space_annotation() {
+ let source = " ";
+ let input = Level::Error.title("title").snippet(
+ Snippet::source(source)
+ .fold(false)
+ .annotation(Level::Error.span(1..1).label("annotation")),
+ );
+ let expected = "\
+error: title
+ |
+1 |\x20\x20
+ | ^ annotation
+ |\
+";
+ let renderer = Renderer::plain();
+ assert_data_eq!(renderer.render(input).to_string(), expected);
+}