Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Fix warning highlight for trailing whitespace #1037

Merged
merged 1 commit into from
May 17, 2023
Merged

Fix warning highlight for trailing whitespace #1037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
91 changes: 71 additions & 20 deletions src/edits.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,33 @@ 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;
}

(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>)],
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -553,7 +577,8 @@ mod tests {
],
],
vec![vec![
(PlusNoop, "á á b á á "),
(PlusNoop, "á á b á á"),
(PlusNoop, " "),
(Insertion, "c"),
(PlusNoop, " á á b"),
]],
Expand All @@ -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,
Expand Down Expand Up @@ -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>("),
]],
Expand All @@ -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, ")"),
Expand All @@ -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,
);
Expand All @@ -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,
);
Expand All @@ -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"),
Expand Down Expand Up @@ -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"],
(
Expand All @@ -783,7 +831,8 @@ mod tests {
(Insertion, "x"),
(Insertion, " "),
(Insertion, "y"),
(PlusNoop, " c "),
(PlusNoop, " c"),
(Insertion, " "),
(Insertion, "z"),
(PlusNoop, " "),
],
Expand All @@ -795,6 +844,7 @@ mod tests {
],
],
),
1.0,
);
}

Expand Down Expand Up @@ -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"),
Expand Down
35 changes: 12 additions & 23 deletions src/paint.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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