Highlight whitespace errors

5 files changed, 149 insertions, 5 deletions

diff --git a/src/cli.rs b/src/cli.rs
index e91b7056..ec45fffb 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -415,6 +415,11 @@ pub struct Opt {
     #[structopt(long = "--plus-empty-line-marker-style", default_value = "normal auto")]
     pub plus_empty_line_marker_style: String,
 
+    /// Style for whitespace errors. Defaults to color.diff.whitespace if that is set in git
+    /// config, or else 'magenta reverse'.
+    #[structopt(long = "whitespace-error-style", default_value = "auto auto")]
+    pub whitespace_error_style: String,
+
     #[structopt(long = "minus-color")]
     /// Deprecated: use --minus-style='normal my_background_color'.
     pub deprecated_minus_background_color: Option<String>,
diff --git a/src/config.rs b/src/config.rs
index 794acc7d..e05d7b8c 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -72,6 +72,7 @@ pub struct Config {
     pub tab_width: usize,
     pub true_color: bool,
     pub tokenization_regex: Regex,
+    pub whitespace_error_style: Style,
     pub zero_style: Style,
 }
 
@@ -215,6 +216,7 @@ impl From<cli::Opt> for Config {
             plus_emph_style,
             plus_non_emph_style,
             plus_empty_line_marker_style,
+            whitespace_error_style,
         ) = make_hunk_styles(&opt, is_light_mode, true_color);
 
         let (commit_style, file_style, hunk_header_style) =
@@ -300,6 +302,7 @@ impl From<cli::Opt> for Config {
             tab_width: opt.tab_width,
             tokenization_regex,
             true_color,
+            whitespace_error_style,
             zero_style,
         }
     }
@@ -319,6 +322,7 @@ fn make_hunk_styles<'a>(
     Style,
     Style,
     Style,
+    Style,
 ) {
     let minus_style = Style::from_str(
         &opt.minus_style,
@@ -416,6 +420,15 @@ fn make_hunk_styles<'a>(
         false,
     );
 
+    let whitespace_error_style = Style::from_str(
+        &opt.whitespace_error_style,
+        None,
+        None,
+        None,
+        true_color,
+        false,
+    );
+
     (
         minus_style,
         minus_emph_style,
@@ -426,6 +439,7 @@ fn make_hunk_styles<'a>(
         plus_emph_style,
         plus_non_emph_style,
         plus_empty_line_marker_style,
+        whitespace_error_style,
     )
 }
 
diff --git a/src/paint.rs b/src/paint.rs
index d9dddd62..7a98cd3d 100644
--- a/src/paint.rs
+++ b/src/paint.rs
@@ -332,13 +332,21 @@ impl<'a> Painter<'a> {
         } else {
             None
         };
-        Self::update_styles(&mut diff_sections.0, minus_non_emph_style);
+        Self::update_styles(
+            &mut diff_sections.0,
+            config.whitespace_error_style,
+            minus_non_emph_style,
+        );
         let plus_non_emph_style = if config.plus_non_emph_style != config.plus_emph_style {
             Some(config.plus_non_emph_style)
         } else {
             None
         };
-        Self::update_styles(&mut diff_sections.1, plus_non_emph_style);
+        Self::update_styles(
+            &mut diff_sections.1,
+            config.whitespace_error_style,
+            plus_non_emph_style,
+        );
         diff_sections
     }
 
@@ -348,15 +356,30 @@ impl<'a> Painter<'a> {
     ///    inferred edit operations and so, if there is a special non-emph style that is
     ///    distinct from the default style, then it should be used for the non-emph style
     ///    sections.
-    fn update_styles(style_sections: &mut Vec<Vec<(Style, &str)>>, non_emph_style: Option<Style>) {
+    /// 2. If the line constitutes a whitespace error, then the whitespace error style
+    ///    should be applied to the added material.
+    fn update_styles(
+        style_sections: &mut Vec<Vec<(Style, &str)>>,
+        whitespace_error_style: Style,
+        non_emph_style: Option<Style>,
+    ) {
         for line_sections in style_sections {
             let line_has_emph_and_non_emph_sections =
                 style_sections_contain_more_than_one_style(line_sections);
             let should_update_non_emph_styles =
                 non_emph_style.is_some() && line_has_emph_and_non_emph_sections;
+            let is_whitespace_error = is_whitespace_error(line_sections);
             for section in line_sections.iter_mut() {
-                // Update the style if this is a non-emph section that needs updating.
-                if should_update_non_emph_styles && !section.0.is_emph {
+                // 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.
+                if is_whitespace_error
+                    && (section.0.is_emph || !line_has_emph_and_non_emph_sections)
+                {
+                    *section = (whitespace_error_style, section.1);
+                }
+                // Otherwise, update the style if this is a non-emph section that needs updating.
+                else if should_update_non_emph_styles && !section.0.is_emph {
                     *section = (non_emph_style.unwrap(), section.1);
                 }
             }
@@ -379,6 +402,20 @@ fn style_sections_contain_more_than_one_style(sections: &Vec<(Style, &str)>) ->
     }
 }
 
+lazy_static! {
+    static ref NON_WHITESPACE_REGEX: Regex = Regex::new(r"\S").unwrap();
+}
+
+/// True iff the line represented by `sections` constitutes a whitespace error.
+// 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: &Vec<(Style, &str)>) -> bool {
+    !sections
+        .iter()
+        .any(|(_, s)| NON_WHITESPACE_REGEX.is_match(s))
+}
+
 mod superimpose_style_sections {
     use syntect::highlighting::Style as SyntectStyle;
 
diff --git a/src/preset.rs b/src/preset.rs
index a2178fb3..7dd13207 100644
--- a/src/preset.rs
+++ b/src/preset.rs
@@ -536,6 +536,81 @@ mod tests {
         remove_file(git_config_path).unwrap();
     }
 
+    #[test]
+    fn test_whitespace_error_style() {
+        let git_config_contents = b"
+[color \"diff\"]
+    whitespace = yellow dim ul magenta
+";
+        let git_config_path = "delta__test_whitespace_error_style.gitconfig";
+
+        // Git config disabled: hard-coded delta default wins
+        assert_eq!(
+            make_config(&[], None, None).whitespace_error_style,
+            make_style("magenta reverse")
+        );
+
+        // Unspecified by user: color.diff.whitespace wins
+        assert_eq!(
+            make_config(&[], Some(git_config_contents), Some(git_config_path))
+                .whitespace_error_style,
+            make_style("yellow dim ul magenta")
+        );
+
+        // Command line argument wins
+        assert_eq!(
+            make_config(
+                &["--whitespace-error-style", "red reverse"],
+                Some(git_config_contents),
+                Some(git_config_path)
+            )
+            .whitespace_error_style,
+            make_style("reverse red")
+        );
+
+        let git_config_contents = b"
+[color \"diff\"]
+    whitespace = yellow dim ul magenta
+
+[delta]
+    whitespace-error-style = blue reverse
+
+[delta \"my-whitespace-error-style-preset\"]
+    whitespace-error-style = green reverse
+";
+
+        // Command line argument wins
+        assert_eq!(
+            make_config(
+                &["--whitespace-error-style", "red reverse"],
+                Some(git_config_contents),
+                Some(git_config_path)
+            )
+            .whitespace_error_style,
+            make_style("reverse red")
+        );
+
+        // No command line argument; main [delta] section wins
+        assert_eq!(
+            make_config(&[], Some(git_config_contents), Some(git_config_path))
+                .whitespace_error_style,
+            make_style("blue reverse")
+        );
+
+        // No command line argument; preset section wins
+        assert_eq!(
+            make_config(
+                &["--presets", "my-whitespace-error-style-preset"],
+                Some(git_config_contents),
+                Some(git_config_path)
+            )
+            .whitespace_error_style,
+            make_style("reverse green")
+        );
+
+        remove_file(git_config_path).unwrap();
+    }
+
     fn make_style(s: &str) -> Style {
         _make_style(s, false)
     }
diff --git a/src/set_options.rs b/src/set_options.rs
index 007cb326..b6d8c0ae 100644
--- a/src/set_options.rs
+++ b/src/set_options.rs
@@ -2,6 +2,7 @@ use std::collections::HashMap;
 use structopt::clap;
 
 use crate::cli;
+use crate::config;
 use crate::git_config::{self, GitConfigGet};
 use crate::preset::{self, GetValueFunctionFromBuiltinPreset};
 
@@ -161,6 +162,17 @@ pub fn set_options(
     if opt.no_gitconfig {
         return;
     }
+    // Handle options which default to an arbitrary git config value.
+    // TODO: incorporate this logic into the set_options macro.
+    if !config::user_supplied_option("whitespace-error-style", arg_matches) {
+        opt.whitespace_error_style = if let Some(git_config) = git_config {
+            git_config.get::<String>("color.diff.whitespace")
+        } else {
+            None
+        }
+        .unwrap_or_else(|| "magenta reverse".to_string())
+    }
+
     set_options!(
         [
             // --presets must be set first
@@ -212,6 +224,7 @@ pub fn set_options(
             ("syntax_theme", Option<String>, syntax_theme),
             ("tabs", usize, tab_width),
             ("true-color", String, true_color),
+            ("whitespace-error-style", String, whitespace_error_style),
             ("width", Option<String>, width),
             ("word-diff-regex", String, tokenization_regex),
             ("zero-style", String, zero_style)