Skip to content

Commit

Permalink
Fix warning highlight for trailing whitespace
Browse files Browse the repository at this point in the history 
Fix dandavison#137
  • Loading branch information
@wescande
wescande committed May 7, 2023
1 parent ce41a39 commit e825a4f
Show file tree
Hide file tree
Showing 5 changed files with 401 additions and 45 deletions.
81 changes: 61 additions & 20 deletions src/edits.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,16 @@ 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)]);
// Split the line if it contains extra whitespace at the end preceded with non whitespace
let line = plus_line.trim_end_matches(&[' ', '\t', '\n']);
if !line.is_empty() && line != plus_line.trim_end_matches('\n') {
annotated_plus_lines.push(vec![
(noop_insertions[plus_index], line),
(noop_insertions[plus_index], &plus_line[line.len()..]),
]);
} else {
annotated_plus_lines.push(vec![(noop_insertions[plus_index], plus_line)]);
}
line_alignment.push((None, Some(plus_index)));
plus_index += 1;
}
Expand Down Expand Up @@ -228,14 +237,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);
let line = plus_section.trim_end_matches(&[' ', '\t', '\n']);
if !line.is_empty() && line != plus_section.trim_end_matches('\n') {
annotated_plus_line.push((op, line));
annotated_plus_line.push((plus_op_prev, &plus_section[line.len()..]));
} else {
annotated_plus_line.push((op, plus_section));
}
minus_op_prev = noop_deletion;
plus_op_prev = noop_insertion;
}
Expand Down Expand Up @@ -553,7 +567,8 @@ mod tests {
],
],
vec![vec![
(PlusNoop, "á á b á á "),
(PlusNoop, "á á b á á"),
(PlusNoop, " "),
(Insertion, "c"),
(PlusNoop, " á á b"),
]],
Expand All @@ -574,9 +589,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,
Expand Down Expand Up @@ -615,7 +640,8 @@ mod tests {
(MinusNoop, "EditOperation>("),
]],
vec![vec![
(PlusNoop, "fn coalesce_edits<'a, "),
(PlusNoop, "fn coalesce_edits<'a,"),
(PlusNoop, " "),
(Insertion, "'b, "),
(PlusNoop, "EditOperation>("),
]],
Expand All @@ -636,7 +662,8 @@ mod tests {
(MinusNoop, ":"),
]],
vec![vec![
(PlusNoop, "for _ in range(0, "),
(PlusNoop, "for _ in range(0,"),
(PlusNoop, " "),
(Insertion, "int("),
(PlusNoop, "options[\"count\"])"),
(Insertion, ")"),
Expand All @@ -654,7 +681,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,
);
Expand All @@ -663,7 +695,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,
);
Expand All @@ -684,7 +721,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"),
Expand Down Expand Up @@ -761,7 +799,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"],
(
Expand All @@ -783,7 +821,8 @@ mod tests {
(Insertion, "x"),
(Insertion, " "),
(Insertion, "y"),
(PlusNoop, " c "),
(PlusNoop, " c"),
(Insertion, " "),
(Insertion, "z"),
(PlusNoop, " "),
],
Expand All @@ -795,6 +834,7 @@ mod tests {
],
],
),
1.0,
);
}

Expand Down Expand Up @@ -834,7 +874,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"),
Expand Down
35 changes: 12 additions & 23 deletions src/paint.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();
}
}
}
}
Expand Down Expand Up @@ -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;

Expand Down
10 changes: 10 additions & 0 deletions src/style.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
73 changes: 71 additions & 2 deletions src/tests/ansi_test_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
Loading

0 comments on commit e825a4f

Please sign in to comment.