Skip to content

Commit

Permalink
renderer: fix rendering of source when it's a single ASCII whitespace
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
BurntSushi committed Jan 8, 2025
1 parent 7132bf3 commit 910bfef
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 3 deletions.
4 changes: 1 addition & 3 deletions src/renderer/display_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
26 changes: 26 additions & 0 deletions tests/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

0 comments on commit 910bfef

Please sign in to comment.