Fix warning highlight for trailing whitespace (#1037)

Fix #137
This commit is contained in:
William Escande
2023-05-17 04:32:25 -07:00
committed by GitHub
parent b3ee8400dd
commit feec45b5d7
5 changed files with 411 additions and 45 deletions

View File

@@ -90,7 +90,14 @@ where
}
// Emit any remaining plus lines
for plus_line in &plus_lines[plus_index..] {
annotated_plus_lines.push(vec![(noop_insertions[plus_index], plus_line)]);
if let Some(content) = get_contents_before_trailing_whitespace(plus_line) {
annotated_plus_lines.push(vec![
(noop_insertions[plus_index], content),
(noop_insertions[plus_index], &plus_line[content.len()..]),
]);
} else {
annotated_plus_lines.push(vec![(noop_insertions[plus_index], plus_line)]);
}
line_alignment.push((None, Some(plus_index)));
plus_index += 1;
}
@@ -98,6 +105,18 @@ where
(annotated_minus_lines, annotated_plus_lines, line_alignment)
}
// Return `None` if there is no trailing whitespace.
// Return `Some(content)` where content is trimmed if there was some trailing whitespace
fn get_contents_before_trailing_whitespace(line: &str) -> Option<&str> {
let content = line.trim_end();
// if line has a trailing newline, do not consider it as a 'trailing whitespace'
if !content.is_empty() && content != line.trim_end_matches('\n') {
Some(content)
} else {
None
}
}
// Return boolean arrays indicating whether each line has a homolog (is "paired").
pub fn make_lines_have_homolog(
line_alignment: &[(Option<usize>, Option<usize>)],
@@ -228,14 +247,19 @@ where
},
minus_section,
));
annotated_plus_line.push((
if coalesce_space_with_previous {
plus_op_prev
} else {
noop_insertion
},
plus_section(n, &mut y_offset),
));
let op = if coalesce_space_with_previous {
plus_op_prev
} else {
noop_insertion
};
let plus_section = plus_section(n, &mut y_offset);
if let Some(non_whitespace) = get_contents_before_trailing_whitespace(plus_section)
{
annotated_plus_line.push((op, non_whitespace));
annotated_plus_line.push((plus_op_prev, &plus_section[non_whitespace.len()..]));
} else {
annotated_plus_line.push((op, plus_section));
}
minus_op_prev = noop_deletion;
plus_op_prev = noop_insertion;
}
@@ -553,7 +577,8 @@ mod tests {
],
],
vec![vec![
(PlusNoop, "á á b á á "),
(PlusNoop, "á á b á á"),
(PlusNoop, " "),
(Insertion, "c"),
(PlusNoop, " á á b"),
]],
@@ -574,9 +599,19 @@ mod tests {
vec![(MinusNoop, "cccc "), (Deletion, "c"), (MinusNoop, " ccc")],
],
vec![
vec![(PlusNoop, "bbbb "), (Insertion, "!"), (PlusNoop, " bbb")],
vec![
(PlusNoop, "bbbb"),
(PlusNoop, " "),
(Insertion, "!"),
(PlusNoop, " bbb"),
],
vec![(PlusNoop, "dddd d ddd")],
vec![(PlusNoop, "cccc "), (Insertion, "!"), (PlusNoop, " ccc")],
vec![
(PlusNoop, "cccc"),
(PlusNoop, " "),
(Insertion, "!"),
(PlusNoop, " ccc"),
],
],
),
0.66,
@@ -615,7 +650,8 @@ mod tests {
(MinusNoop, "EditOperation>("),
]],
vec![vec![
(PlusNoop, "fn coalesce_edits<'a, "),
(PlusNoop, "fn coalesce_edits<'a,"),
(PlusNoop, " "),
(Insertion, "'b, "),
(PlusNoop, "EditOperation>("),
]],
@@ -636,7 +672,8 @@ mod tests {
(MinusNoop, ":"),
]],
vec![vec![
(PlusNoop, "for _ in range(0, "),
(PlusNoop, "for _ in range(0,"),
(PlusNoop, " "),
(Insertion, "int("),
(PlusNoop, "options[\"count\"])"),
(Insertion, ")"),
@@ -654,7 +691,12 @@ mod tests {
vec!["a b a"],
(
vec![vec![(MinusNoop, "a "), (MinusNoop, "a")]],
vec![vec![(PlusNoop, "a "), (Insertion, "b "), (PlusNoop, "a")]],
vec![vec![
(PlusNoop, "a"),
(PlusNoop, " "),
(Insertion, "b "),
(PlusNoop, "a"),
]],
),
1.0,
);
@@ -663,7 +705,12 @@ mod tests {
vec!["a b b a"],
(
vec![vec![(MinusNoop, "a "), (MinusNoop, "a")]],
vec![vec![(PlusNoop, "a "), (Insertion, "b b "), (PlusNoop, "a")]],
vec![vec![
(PlusNoop, "a"),
(PlusNoop, " "),
(Insertion, "b b "),
(PlusNoop, "a"),
]],
),
1.0,
);
@@ -684,7 +731,8 @@ mod tests {
(MinusNoop, " from any one of them."),
]],
vec![vec![
(PlusNoop, "so it is safe to read "),
(PlusNoop, "so it is safe to read"),
(PlusNoop, " "),
(Insertion, "build"),
(Insertion, " "),
(Insertion, "info"),
@@ -761,7 +809,7 @@ mod tests {
#[test]
fn test_infer_edits_14() {
assert_paired_edits(
assert_edits(
vec!["a b c d ", "p "],
vec!["x y c z ", "q r"],
(
@@ -783,7 +831,8 @@ mod tests {
(Insertion, "x"),
(Insertion, " "),
(Insertion, "y"),
(PlusNoop, " c "),
(PlusNoop, " c"),
(Insertion, " "),
(Insertion, "z"),
(PlusNoop, " "),
],
@@ -795,6 +844,7 @@ mod tests {
],
],
),
1.0,
);
}
@@ -834,7 +884,8 @@ mod tests {
vec![vec![
(PlusNoop, ""),
(Insertion, "c "),
(PlusNoop, "a a a a a a "),
(PlusNoop, "a a a a a a"),
(Insertion, " "),
(Insertion, "c"),
(Insertion, " "),
(Insertion, "c"),

View File

@@ -522,9 +522,15 @@ impl<'p> Painter<'p> {
let line_has_emph_and_non_emph_sections =
style_sections_contain_more_than_one_style(style_sections);
let should_update_non_emph_styles = non_emph_style.is_some() && *line_has_homolog;
let is_whitespace_error =
whitespace_error_style.is_some() && is_whitespace_error(style_sections);
for (style, _) in style_sections.iter_mut() {
// TODO: Git recognizes blank lines at end of file (blank-at-eof)
// as a whitespace error but delta does not yet.
// https://git-scm.com/docs/git-config#Documentation/git-config.txt-corewhitespace
let mut is_whitespace_error = whitespace_error_style.is_some();
for (style, s) in style_sections.iter_mut().rev() {
if is_whitespace_error && !s.trim().is_empty() {
is_whitespace_error = false;
}
// If the line as a whole constitutes a whitespace error then highlight this
// section if either (a) it is an emph section, or (b) the line lacks any
// emph/non-emph distinction.
@@ -537,6 +543,9 @@ impl<'p> Painter<'p> {
// Otherwise, update the style if this is a non-emph section that needs updating.
else if should_update_non_emph_styles && !style.is_emph {
*style = non_emph_style.unwrap();
if is_whitespace_error {
*style = whitespace_error_style.unwrap();
}
}
}
}
@@ -856,26 +865,6 @@ fn style_sections_contain_more_than_one_style(sections: &[(Style, &str)]) -> boo
}
}
/// True iff the line represented by `sections` constitutes a whitespace error.
// A line is a whitespace error iff it is non-empty and contains only whitespace
// characters.
// TODO: Git recognizes blank lines at end of file (blank-at-eof)
// as a whitespace error but delta does not yet.
// https://git-scm.com/docs/git-config#Documentation/git-config.txt-corewhitespace
fn is_whitespace_error(sections: &[(Style, &str)]) -> bool {
let mut any_chars = false;
for c in sections.iter().flat_map(|(_, s)| s.chars()) {
if c == '\n' {
return any_chars;
} else if c != ' ' && c != '\t' {
return false;
} else {
any_chars = true;
}
}
false
}
mod superimpose_style_sections {
use syntect::highlighting::Style as SyntectStyle;

View File

@@ -125,6 +125,16 @@ impl Style {
}
}
#[cfg(test)]
pub fn get_matching_substring<'a>(&self, s: &'a str) -> Option<&'a str> {
for (parsed_style, parsed_str) in ansi::parse_style_sections(s) {
if ansi_term_style_equality(parsed_style, self.ansi_term_style) {
return Some(parsed_str);
}
}
None
}
pub fn to_painted_string(self) -> ansi_term::ANSIGenericString<'static, str> {
self.paint(self.to_string())
}

View File

@@ -8,6 +8,8 @@ pub mod ansi_test_utils {
use crate::paint;
use crate::style::Style;
// Check if `output[line_number]` start with `expected_prefix`
// Then check if the first style in the line is the `expected_style`
pub fn assert_line_has_style(
output: &str,
line_number: usize,
@@ -25,6 +27,50 @@ pub mod ansi_test_utils {
));
}
// Check if `output[line_number]` start with `expected_prefix`
// Then check if it contains the `expected_substring` with the corresponding `expected_style`
// If the line contains multiples times the `expected_style`, will only compare with the first
// item found
pub fn assert_line_contain_substring_style(
output: &str,
line_number: usize,
expected_prefix: &str,
expected_substring: &str,
expected_style: &str,
config: &Config,
) {
assert_eq!(
expected_substring,
_line_get_substring_matching_style(
output,
line_number,
expected_prefix,
expected_style,
config,
)
.unwrap()
);
}
// Check if `output[line_number]` start with `expected_prefix`
// Then check if the line does not contains the `expected_style`
pub fn assert_line_does_not_contain_substring_style(
output: &str,
line_number: usize,
expected_prefix: &str,
expected_style: &str,
config: &Config,
) {
assert!(_line_get_substring_matching_style(
output,
line_number,
expected_prefix,
expected_style,
config,
)
.is_none());
}
pub fn assert_line_does_not_have_style(
output: &str,
line_number: usize,
@@ -150,6 +196,30 @@ pub mod ansi_test_utils {
output_buffer
}
fn _line_extract<'a>(output: &'a str, line_number: usize, expected_prefix: &str) -> &'a str {
let line = output.lines().nth(line_number).unwrap();
assert!(ansi::strip_ansi_codes(line).starts_with(expected_prefix));
line
}
fn _line_get_substring_matching_style<'a>(
output: &'a str,
line_number: usize,
expected_prefix: &str,
expected_style: &str,
config: &Config,
) -> Option<&'a str> {
let line = _line_extract(output, line_number, expected_prefix);
let style = Style::from_str(
expected_style,
None,
None,
config.true_color,
config.git_config.as_ref(),
);
style.get_matching_substring(line)
}
fn _line_has_style(
output: &str,
line_number: usize,
@@ -158,8 +228,7 @@ pub mod ansi_test_utils {
config: &Config,
_4_bit_color: bool,
) -> bool {
let line = output.lines().nth(line_number).unwrap();
assert!(ansi::strip_ansi_codes(line).starts_with(expected_prefix));
let line = _line_extract(output, line_number, expected_prefix);
let mut style = Style::from_str(
expected_style,
None,

View File

@@ -1476,6 +1476,182 @@ src/align.rs:71: impl<'a> Alignment<'a> { │
);
}
#[test]
fn test_whitespace_unrelated_edit_text_error() {
let whitespace_error_style = "bold yellow red ul";
let config = integration_test_utils::make_config_from_args(&[
"--whitespace-error-style",
whitespace_error_style,
]);
let output =
integration_test_utils::run_delta(DIFF_WITH_WHITESPACE_UNRELATED_EDIT_ERROR, &config);
ansi_test_utils::assert_line_contain_substring_style(
&output,
9,
"some new",
" ",
whitespace_error_style,
&config,
);
}
#[test]
fn test_whitespace_edit_text_error() {
let whitespace_error_style = "bold yellow red ul";
let config = integration_test_utils::make_config_from_args(&[
"--whitespace-error-style",
whitespace_error_style,
]);
let output = integration_test_utils::run_delta(DIFF_WITH_WHITESPACE_EDIT_ERROR, &config);
ansi_test_utils::assert_line_contain_substring_style(
&output,
9,
"same ",
" ",
whitespace_error_style,
&config,
);
}
#[test]
fn test_whitespace_added_empty_line_error() {
let whitespace_error_style = "bold yellow red ul";
let config = integration_test_utils::make_config_from_args(&[
"--whitespace-error-style",
whitespace_error_style,
]);
let output =
integration_test_utils::run_delta(DIFF_WITH_ADDED_WHITESPACE_EMPTY_LINE_ERROR, &config);
// TODO is this the first style ?
ansi_test_utils::assert_line_has_style(&output, 9, " ", whitespace_error_style, &config);
ansi_test_utils::assert_line_contain_substring_style(
&output,
9,
"",
" ",
whitespace_error_style,
&config,
);
}
#[test]
fn test_whitespace_after_text_error() {
let whitespace_error_style = "bold yellow red ul";
let config = integration_test_utils::make_config_from_args(&[
"--whitespace-error-style",
whitespace_error_style,
]);
let output =
integration_test_utils::run_delta(DIFF_WITH_WHITESPACE_AFTER_TEXT_ERROR, &config);
ansi_test_utils::assert_line_contain_substring_style(
&output,
8,
"foo bar",
" ",
whitespace_error_style,
&config,
);
let output = integration_test_utils::run_delta(
DIFF_WITH_REMOVED_WHITESPACE_AFTER_TEXT_ERROR,
&config,
);
ansi_test_utils::assert_line_does_not_contain_substring_style(
&output,
8,
"foo bar",
whitespace_error_style,
&config,
);
}
#[test]
fn test_whitespace_complex_error() {
let whitespace_error_style = "bold yellow red ul";
let config = integration_test_utils::make_config_from_args(&[
"--whitespace-error-style",
whitespace_error_style,
]);
let output = integration_test_utils::run_delta(DIFF_WITH_WHITESPACE_COMPLEX_ERROR, &config);
// `minus` line should not display whitespace error
ansi_test_utils::assert_line_does_not_have_style(
&output,
8,
" ",
whitespace_error_style,
&config,
);
ansi_test_utils::assert_line_does_not_have_style(
&output,
9,
" ",
whitespace_error_style,
&config,
);
ansi_test_utils::assert_line_does_not_contain_substring_style(
&output,
10,
" foo0 ",
whitespace_error_style,
&config,
);
ansi_test_utils::assert_line_does_not_contain_substring_style(
&output,
11,
" foo1 ",
whitespace_error_style,
&config,
);
ansi_test_utils::assert_line_does_not_contain_substring_style(
&output,
12,
" bar ",
whitespace_error_style,
&config,
);
// `plus` line should display whitespace error
ansi_test_utils::assert_line_contain_substring_style(
&output,
13,
" ",
" ",
whitespace_error_style,
&config,
);
ansi_test_utils::assert_line_contain_substring_style(
&output,
14,
" ",
" ",
whitespace_error_style,
&config,
);
ansi_test_utils::assert_line_contain_substring_style(
&output,
15,
" foo0",
" ",
whitespace_error_style,
&config,
);
ansi_test_utils::assert_line_contain_substring_style(
&output,
16,
" foo1",
" ",
whitespace_error_style,
&config,
);
ansi_test_utils::assert_line_contain_substring_style(
&output,
17,
" bAr",
" ",
whitespace_error_style,
&config,
);
}
#[test]
fn test_added_empty_line_is_not_whitespace_error() {
let plus_style = "bold yellow red ul";
@@ -2292,6 +2468,77 @@ index 8d1c8b6..8b13789 100644
@@ -1 +1 @@
-
+
";
const DIFF_WITH_WHITESPACE_UNRELATED_EDIT_ERROR: &str = r"
diff --git a/foo b/foo
index 8d1c8b6..8b13789 100644
--- a/foo
+++ b/foo
@@ -1 +1 @@
-some line with trailing spaces
+some new line with trailing spaces
";
const DIFF_WITH_WHITESPACE_EDIT_ERROR: &str = r"
diff --git a/foo b/foo
index 8d1c8b6..8b13789 100644
--- a/foo
+++ b/foo
@@ -1 +1 @@
-same line with different number of trailing spaces
+same line with different number of trailing spaces
";
const DIFF_WITH_WHITESPACE_AFTER_TEXT_ERROR: &str = r"
diff --git c/a i/a
new file mode 100644
index 0000000..8d1c8b6
--- /dev/null
+++ i/a
@@ -0,0 +1 @@
+foo bar
";
const DIFF_WITH_REMOVED_WHITESPACE_AFTER_TEXT_ERROR: &str = r"
diff --git i/a w/a
index 8d1c8b6..8b13789 100644
--- i/a
+++ w/a
@@ -1 +0,0 @@
-foo bar
";
const DIFF_WITH_ADDED_WHITESPACE_EMPTY_LINE_ERROR: &str = r"
diff --git a/a b/a
index 0ec702f..8c75341 100644
--- a/a
+++ b/a
@@ -1,0 +1,0 @@
-
+
";
// Delta handling is different for each of theses cases:
// * Only space in the line is added or partially removed
// * Space after text added or partially removed
// * Space in a unmodified part of the line
// This test regroup theses 5 cases.
const DIFF_WITH_WHITESPACE_COMPLEX_ERROR: &str = r"
diff --git a/a b/a
index 0ec702f..8c75341 100644
--- a/a
+++ b/a
@@ -1,5 +1,5 @@
-
-
- foo0
- foo1
- bar
+
+
+ foo0
+ foo1
+ bAr
";
const DIFF_WITH_TWO_ADDED_LINES: &str = r#"