8000 fix: Color suggestion removal diff by Muscraft · Pull Request #226 · rust-lang/annotate-snippets-rs · GitHub
[go: up one dir, main page]

Skip to content

fix: Color suggestion removal diff #226

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions examples/custom_level.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
88 changes: 81 additions & 7 deletions src/renderer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1874,6 +1874,7 @@ impl Renderer {
show_code_change
{
for part in parts {
let snippet = sm.span_to_snippet(part.span.clone()).unwrap_or_default();
let (span_start, span_end) = sm.span_to_locations(part.span.clone());
let span_start_pos = span_start.display;
let span_end_pos = span_end.display;
Expand Down Expand Up @@ -1932,13 +1933,86 @@ impl Renderer {
}
if let DisplaySuggestion::Diff = show_code_change {
// Colorize removal with red in diff format.
buffer.set_style_range(
row_num - 2,
(padding as isize + span_start_pos as isize) as usize,
(padding as isize + span_end_pos as isize) as usize,
ElementStyle::Removal,
true,
);

// Below, there's some tricky buffer indexing going on. `row_num` at this
// point corresponds to:
//
// |
// LL | CODE
// | ++++ <- `row_num`
//
// in the buffer. When we have a diff format output, we end up with
//
// |
// LL - OLDER <- row_num - 2
// LL + NEWER
// | <- row_num
//
// The `row_num - 2` is to select the buffer line that has the "old version
// of the diff" at that point. When the removal is a single line, `i` is
// `0`, `newlines` is `1` so `(newlines - i - 1)` ends up being `0`, so row
// points at `LL - OLDER`. When the removal corresponds to multiple lines,
// we end up with `newlines > 1` and `i` being `0..newlines - 1`.
//
8000 // |
// LL - OLDER <- row_num - 2 - (newlines - last_i - 1)
// LL - CODE
// LL - BEING
// LL - REMOVED <- row_num - 2 - (newlines - first_i - 1)
// LL + NEWER
// | <- row_num

let newlines = snippet.lines().count();
if newlines > 0 && row_num > newlines {
// Account for removals where the part being removed spans multiple
// lines.
// FIXME: We check the number of rows because in some cases, like in
// `tests/ui/lint/invalid-nan-comparison-suggestion.rs`, the rendered
// suggestion will only show the first line of code being replaced. The
// proper way of doing this would be to change the suggestion rendering
// logic to show the whole prior snippet, but the current output is not
// too bad to begin with, so we side-step that issue here.
for (i, line) in snippet.lines().enumerate() {
let line = normalize_whitespace(line);
let row = row_num - 2 - (newlines - i - 1);
// On the first line, we highlight between the start of the part
// span, and the end of that line.
// On the last line, we highlight between the start of the line, and
// the column of the part span end.
// On all others, we highlight the whole line.
let start = if i == 0 {
(padding as isize + span_start_pos as isize) as usize
} else {
padding
};
let end = if i == 0 {
(padding as isize
+ span_start_pos as isize
+ line.len() as isize)
as usize
} else if i == newlines - 1 {
(padding as isize + span_end_pos as isize) as usize
} else {
(padding as isize + line.len() as isize) as usize
};
buffer.set_style_range(
row,
start,
end,
ElementStyle::Removal,
true,
);
}
} else {
// The removed code fits all in one line.
buffer.set_style_range(
row_num - 2,
(padding as isize + span_start_pos as isize) as usize,
(padding as isize + span_end_pos as isize) as usize,
ElementStyle::Removal,
true,
);
}
}

// length of the code after substitution
Expand Down
1 change: 1 addition & 0 deletions tests/color/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ mod fold_bad_origin_line;
mod fold_leading;
mod fold_trailing;
mod issue_9;
mod multiline_removal_suggestion;
mod multiple_annotations;
mod simple;
mod strip_line;
Expand Down
113 changes: 113 additions & 0 deletions tests/color/multiline_removal_suggestion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
use annotate_snippets::{AnnotationKind, Group, Level, Origin, Patch, Renderer, Snippet};

use snapbox::{assert_data_eq, file};

#[test]
fn case() {
let source = r#"// Make sure suggestion for removal of a span that covers multiple lines is properly highlighted.
//@ compile-flags: --error-format=human --color=always
//@ edition:2018
//@ only-linux
// ignore-tidy-tab
// We use `\t` instead of spaces for indentation to ensure that the highlighting logic properly
// accounts for replaced characters (like we do for `\t` with ` `). The naïve way of highlighting
// could be counting chars of the original code, instead of operating on the code as it is being
// displayed.
use std::collections::{HashMap, HashSet};
fn foo() -> Vec<(bool, HashSet<u8>)> {
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
hm.into_iter()
.map(|(is_true, ts)| {
ts.into_iter()
.map(|t| {
(
is_true,
t,
)
}).flatten()
})
.flatten()
.collect()
}
fn bar() -> Vec<(bool, HashSet<u8>)> {
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
hm.into_iter()
.map(|(is_true, ts)| {
ts.into_iter()
.map(|t| (is_true, t))
.flatten()
})
.flatten()
.collect()
}
fn baz() -> Vec<(bool, HashSet<u8>)> {
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
hm.into_iter()
.map(|(is_true, ts)| {
ts.into_iter().map(|t| {
(is_true, t)
}).flatten()
})
.flatten()
.collect()
}
fn bay() -> Vec<(bool, HashSet<u8>)> {
let mut hm = HashMap::<bool, Vec<HashSet<u8>>>::new();
hm.into_iter()
.map(|(is_true, ts)| {
ts.into_iter()
.map(|t| (is_true, t)).flatten()
})
.flatten()
.collect()
}
fn main() {}
"#;

let input = Level::ERROR
.header("`(bool, HashSet<u8>)` is not an iterator")
.id("E0277")
.group(
Group::new()
.element(
Snippet::source(source)
.origin("$DIR/multiline-removal-suggestion.rs")
.fold(true)
.annotation(
AnnotationKind::Primary
.span(769..776)
.label("`(bool, HashSet<u8>)` is not an iterator"),
),
)
.element(
Level::HELP
.title("the trait `Iterator` is not implemented for `(bool, HashSet<u8>)`"),
)
.element(
Level::NOTE
.title("required for `(bool, HashSet<u8>)` to implement `IntoIterator`"),
),
)
.group(
Group::new()
.element(Level::NOTE.title("required by a bound in `flatten`"))
.element(
Origin::new("/rustc/FAKE_PREFIX/library/core/src/iter/traits/iterator.rs")
.line(1556)
.char_column(4),
),
)
.group(
Group::new()
.element(Level::HELP.title("consider removing this method call, as the receiver has type `std::vec::IntoIter<HashSet<u8>>` and `std::vec::IntoIter<HashSet<u8>>: Iterator` trivially holds"))
.element(
Snippet::source(source)
.origin("$DIR/multiline-removal-suggestion.rs")
.fold(true)
.patch(Patch::new(708..768, "")),
),
< 6D40 span class='blob-code-inner blob-code-marker ' data-code-marker="+"> );
let expected = file!["multiline_removal_suggestion.term.svg"];
let renderer = Renderer::styled();
assert_data_eq!(renderer.render(input), expected);
}
68 changes: 68 additions & 0 deletions tests/color/multiline_removal_suggestion.term.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
0