fix: bg:none overwrites foreground colour (#1903)

Changes the behaviour of a bg:none color string so that
it causes the background to have no color instead of
decoloring the entire string like it did before.

2 files changed, 55 insertions, 9 deletions

diff --git a/docs/advanced-config/README.md b/docs/advanced-config/README.md
index 21ae60a27..ed6bb7c0f 100644
--- a/docs/advanced-config/README.md
+++ b/docs/advanced-config/README.md
@@ -99,7 +99,7 @@ Style strings are a list of words, separated by whitespace. The words are not ca
 
 where `<color>` is a color specifier (discussed below). `fg:<color>` and `<color>` currently do the same thing , though this may change in the future. The order of words in the string does not matter.
 
-The `none` token overrides all other tokens in a string, so that e.g. `fg:red none fg:blue` will still create a string with no styling. It may become an error to use `none` in conjunction with other tokens in the future.
+The `none` token overrides all other tokens in a string if it is not part of a `bg:` specifier, so that e.g. `fg:red none fg:blue` will still create a string with no styling. `bg:none`  sets the background to the default color so `fg:red bg:none` is equivalent to `red` or `fg:red` and `bg:green fg:red bg:none` is also equivalent to `fg:red` or `red`. It may become an error to use `none` in conjunction with other tokens in the future.
 
 A color specifier can be one of the following:
 
diff --git a/src/config.rs b/src/config.rs
index cc3beaa5a..a210f52e8 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -375,16 +375,31 @@ pub fn parse_style_string(style_string: &str) -> Option<ansi_term::Style> {
                     "bold" => Some(style.bold()),
                     "italic" => Some(style.italic()),
                     "dimmed" => Some(style.dimmed()),
-                    "none" => None,
-
-                    // Try to see if this token parses as a valid color string
-                    color_string => parse_color_string(color_string).map(|ansi_color| {
-                        if col_fg {
-                            style.fg(ansi_color)
+                    // When the string is supposed to be a color:
+                    // Decide if we yield none, reset background or set color.
+                    color_string => {
+                        if color_string == "none" && col_fg {
+                            None // fg:none yields no style.
                         } else {
-                            style.on(ansi_color)
+                            // Either bg or valid color or both.
+                            let parsed = parse_color_string(color_string);
+                            // bg + invalid color = reset the background to default.
+                            if !col_fg && parsed.is_none() {
+                                let mut new_style = style;
+                                new_style.background = Option::None;
+                                Some(new_style)
+                            } else {
+                                // Valid color, apply color to either bg or fg
+                                parsed.map(|ansi_color| {
+                                    if col_fg {
+                                        style.fg(ansi_color)
+                                    } else {
+                                        style.on(ansi_color)
+                                    }
+                                })
+                            }
                         }
-                    }),
+                    }
                 }
             })
         })
@@ -685,6 +700,37 @@ mod tests {
     }
 
     #[test]
+    fn table_get_styles_with_none() {
+        // Test that none on the end will result in None, overriding bg:none
+        let config = Value::from("fg:red bg:none none");
+        assert!(<Style>::from_config(&config).is_none());
+
+        // Test that none in front will result in None, overriding bg:none
+        let config = Value::from("none fg:red bg:none");
+        assert!(<Style>::from_config(&config).is_none());
+
+        // Test that none in the middle will result in None, overriding bg:none
+        let config = Value::from("fg:red none bg:none");
+        assert!(<Style>::from_config(&config).is_none());
+
+        // Test that fg:none will result in None
+        let config = Value::from("fg:none bg:black");
+        assert!(<Style>::from_config(&config).is_none());
+
+        // Test that bg:none will yield a style
+        let config = Value::from("fg:red bg:none");
+        assert_eq!(<Style>::from_config(&config).unwrap(), Color::Red.normal());
+
+        // Test that bg:none will yield a style
+        let config = Value::from("fg:red bg:none bold");
+        assert_eq!(<Style>::from_config(&config).unwrap(), Color::Red.bold());
+
+        // Test that bg:none will overwrite the previous background colour
+        let config = Value::from("fg:red bg:green bold bg:none");
+        assert_eq!(<Style>::from_config(&config).unwrap(), Color::Red.bold());
+    }
+
+    #[test]
     fn table_get_styles_ordered() {
         // Test a background style with inverted order (also test hex + ANSI)
         let config = Value::from("bg:#050505 underline fg:120");