Fix warning highlight for trailing whitespace (dandavison#1037)

Fix dandavison#137

src/edits.rs

@@ -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>)],
@@ -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"),

src/paint.rs

@@ -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;
 

src/style.rs

@@ -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())
     }

src/tests/ansi_test_utils.rs

@@ -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,