Fix warning highlight for trailing whitespace (#1037)

Fix #137

5 files changed, 411 insertions, 45 deletions

diff --git a/src/edits.rs b/src/edits.rs
index eb6a310a..b6e1ad9f 100644
--- a/src/edits.rs
+++ b/src/edits.rs
@@ -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"),
diff --git a/src/paint.rs b/src/paint.rs
index 67bd58c7..ca35aa54 100644
--- a/src/paint.rs
+++ b/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;
 
diff --git a/src/style.rs b/src/style.rs
index aaf42730..c52400c8 100644
--- a/src/style.rs
+++ b/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())
     }
diff --git a/src/tests/ansi_test_utils.rs b/src/tests/ansi_test_utils.rs
index 083cf58b..37a7b69c 100644
--- a/src/tests/ansi_test_utils.rs
+++ b/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,
diff --git a/src/tests/test_example_diffs.rs b/src/tests/test_example_diffs.rs
index b3595149..56f6fc09 100644
--- a/src/tests/test_example_diffs.rs
+++ b/src/tests/test_example_diffs.rs
@@ -1477,6 +1477,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";
         let config = integration_test_utils::make_config_from_args(&[
@@ -2294,6 +2470,77 @@ index 8d1c8b6..8b13789 100644
 +
 ";
 
+    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#"
 diff --git a/example.c b/example.c
 index 386f291a..22666f79 100644