From b1c4112e9bc360b679564479f985057cef00515f Mon Sep 17 00:00:00 2001 From: Andrew Gallant 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); +}