-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Description
Following #19919 and #20443, we now render fix diffs in the default output format for both the linter and formatter (in preview). For the snippet attached to the diagnostic, we use our fork of annotate-snippets to render the code but separate code for rendering the fix diff. We did our best to get the two to align well, and I think most cases are covered, but there are a few examples where it breaks down. For example, we render more context lines in the diff than in the snippet, so changes near a line width boundary (e.g. 9 -> 10, 99 -> 100) can result in a shift of the line number separator before and after the help message:
ruff/crates/ruff_db/src/diagnostic/render/full.rs
Lines 944 to 963 in b483d3b
| error[test-diagnostic][*]: main diagnostic message | |
| --> example.py:3:1 | |
| | | |
| 1 | line 1 | |
| 2 | line 2 | |
| 3 | line 3 | |
| | ^^^^^^ label | |
| 4 | line 4 | |
| 5 | line 5 | |
| | | |
| help: Start of diff: | |
| 4 | line 4 | |
| 5 | line 5 | |
| 6 | line 6 | |
| - line 7 | |
| 7 + fixed line 7 | |
| 8 | line 8 | |
| 9 | line 9 | |
| 10 | line 10 | |
| note: This is an unsafe fix and may change runtime behavior |
This is somewhat more of an issue in the formatter because we don't want to render a snippet at all. We thus added a manual header_offset field in #20443, but it would be nice to reuse the same line width calculation in both steps.
A more robust approach to diff rendering would be to incorporate it into annotate-snippets directly. The line number width is computed here:
ruff/crates/ruff_annotate_snippets/src/renderer/display_list.rs
Lines 79 to 84 in bedfc6f
| let lineno_width = self.body.iter().fold(0, |max, set| { | |
| set.display_lines.iter().fold(max, |max, line| match line { | |
| DisplayLine::Source { lineno, .. } => cmp::max(lineno.unwrap_or(0), max), | |
| _ => max, | |
| }) | |
| }); |
based on the maximum line number in the DisplayLine::Source variants. Without looking into it too deeply, I think the ideal solution would be to add a new DisplayLine::Diff variant and incorporate our current Diff type's rendering code into annotate-snippets.
I think this would probably rule out #20411, so we'd also need to commit to staying on our fork.