formatter: use crossterm for colors

Let's use `crossterm` to make `ColorFormatter` a little more readable,
and maybe also more portable.

This uses the `SetForegroundColor()` function, which uses the escapes
for 256-color support (code 38) instead of the 8-color escapes (codes
30-37) combined with bold/bright (code 1) we were using before. IIUC,
most terminals support the 16 base colors when using the 256-color
escape even if they don't support all the 256 colors. It seems like an
improvement to use actual color codes for the bright colors too,
instead of assuming that terminals render bold as bright (even though
most terminals do).

Before this commit, we relied on ANSI escape 1 - which is specified to
make the font bold - to make the color brighter. That's why we call
the colors "bright blue" etc. When we switch from using code 30-37 to
using 38 to let our color config just control the color (not using
escape1), we therefore lose the bold font on many terminals (at least
in iTerm2 and in the terminal application on my Debian work
computer). As a workaround, I made us still use escape 1 when the
bright colors are used. I'll make boldness a separately configurable
attribute soon. Then we'll be able to remove this hack.

With the switch to `crossterm`, we also reset just the foreground
color (code 39) instead of resetting all attributes (code 0). That
also seems like an improvement, probably making it easier for us to
later support different background colors, underlining, etc.

src/formatter.rs

@@ -18,6 +18,9 @@ use std::io::{Error, Write};
 use std::sync::Arc;
 use std::{fmt, io};
 
+use crossterm::queue;
+use crossterm::style::{Attribute, Color, SetAttribute, SetForegroundColor};
+
 // Lets the caller label strings and translates the labels to colors
 pub trait Formatter: Write {
     fn write_bytes(&mut self, data: &[u8]) -> io::Result<()> {
@@ -150,8 +153,8 @@ pub struct ColorFormatter<W> {
     output: W,
     colors: Arc<HashMap<String, String>>,
     labels: Vec<String>,
-    cached_colors: HashMap<Vec<String>, Vec<u8>>,
+    cached_colors: HashMap<Vec<String>, Color>,
-    current_color: Vec<u8>,
+    current_color: Color,
 }
 
 fn config_colors(config: &config::Config) -> HashMap<String, String> {
@@ -171,13 +174,13 @@ impl<W: Write> ColorFormatter<W> {
             colors,
             labels: vec![],
             cached_colors: HashMap::new(),
-            current_color: b"\x1b[0m".to_vec(),
+            current_color: Color::Reset,
         }
     }
 
-    fn current_color(&mut self) -> Vec<u8> {
+    fn current_color(&mut self) -> Color {
         if let Some(cached) = self.cached_colors.get(&self.labels) {
-            cached.clone()
+            *cached
         } else {
             let mut best_match = (-1, "");
             for (key, value) in self.colors.as_ref() {
@@ -209,8 +212,7 @@ impl<W: Write> ColorFormatter<W> {
             }
 
             let color = color_for_name(best_match.1);
-            self.cached_colors
+            self.cached_colors.insert(self.labels.clone(), color);
-                .insert(self.labels.clone(), color.clone());
             color
         }
     }
@@ -218,32 +220,55 @@ impl<W: Write> ColorFormatter<W> {
     fn write_new_color(&mut self) -> io::Result<()> {
         let new_color = self.current_color();
         if new_color != self.current_color {
-            self.output.write_all(&new_color)?;
+            // For now, make bright colors imply bold font. That better matches our
+            // behavior from when we used ANSI codes 30-37 plus an optional 1 for
+            // bold/bright (we now use code 38 for setting foreground color).
+            // TODO: Make boldness configurable separately from color
+            if !is_bright(&self.current_color) && is_bright(&new_color) {
+                queue!(self.output, SetAttribute(Attribute::Bold))?;
+            } else if !is_bright(&new_color) && is_bright(&self.current_color) {
+                queue!(self.output, SetAttribute(Attribute::Reset))?;
+            }
+            queue!(self.output, SetForegroundColor(new_color))?;
             self.current_color = new_color;
         }
         Ok(())
     }
 }
 
-fn color_for_name(color_name: &str) -> Vec<u8> {
+fn is_bright(color: &Color) -> bool {
+    matches!(
+        color,
+        Color::DarkGrey
+            | Color::Red
+            | Color::Green
+            | Color::Yellow
+            | Color::Blue
+            | Color::Magenta
+            | Color::Cyan
+            | Color::White
+    )
+}
+
+fn color_for_name(color_name: &str) -> Color {
     match color_name {
-        "black" => b"\x1b[30m".to_vec(),
+        "black" => Color::Black,
-        "red" => b"\x1b[31m".to_vec(),
+        "red" => Color::DarkRed,
-        "green" => b"\x1b[32m".to_vec(),
+        "green" => Color::DarkGreen,
-        "yellow" => b"\x1b[33m".to_vec(),
+        "yellow" => Color::DarkYellow,
-        "blue" => b"\x1b[34m".to_vec(),
+        "blue" => Color::DarkBlue,
-        "magenta" => b"\x1b[35m".to_vec(),
+        "magenta" => Color::DarkMagenta,
-        "cyan" => b"\x1b[36m".to_vec(),
+        "cyan" => Color::DarkCyan,
-        "white" => b"\x1b[37m".to_vec(),
+        "white" => Color::Grey,
-        "bright black" => b"\x1b[1;30m".to_vec(),
+        "bright black" => Color::DarkGrey,
-        "bright red" => b"\x1b[1;31m".to_vec(),
+        "bright red" => Color::Red,
-        "bright green" => b"\x1b[1;32m".to_vec(),
+        "bright green" => Color::Green,
-        "bright yellow" => b"\x1b[1;33m".to_vec(),
+        "bright yellow" => Color::Yellow,
-        "bright blue" => b"\x1b[1;34m".to_vec(),
+        "bright blue" => Color::Blue,
-        "bright magenta" => b"\x1b[1;35m".to_vec(),
+        "bright magenta" => Color::Magenta,
-        "bright cyan" => b"\x1b[1;36m".to_vec(),
+        "bright cyan" => Color::Cyan,
-        "bright white" => b"\x1b[1;37m".to_vec(),
+        "bright white" => Color::White,
-        _ => b"\x1b[0m".to_vec(),
+        _ => Color::Reset,
     }
 }
 
@@ -321,22 +346,22 @@ mod tests {
             formatter.write_str("\n").unwrap();
         }
         insta::assert_snapshot!(String::from_utf8(output).unwrap(), @r###"
-         black 
+         black 
-         red 
+         red 
-         green 
+         green 
-         yellow 
+         yellow 
-         blue 
+         blue 
-         magenta 
+         magenta 
-         cyan 
+         cyan 
-         white 
+         white 
-         bright black 
+         bright black 
-         bright red 
+         bright red 
-         bright green 
+         bright green 
-         bright yellow 
+         bright yellow 
-         bright blue 
+         bright blue 
-         bright magenta 
+         bright magenta 
-         bright cyan 
+         bright cyan 
-         bright white 
+         bright white 
         "###);
     }
 
@@ -356,7 +381,7 @@ mod tests {
         formatter.write_str(" inside ").unwrap();
         formatter.remove_label().unwrap();
         formatter.write_str(" after ").unwrap();
-        insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" before  inside  after ");
+        insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" before  inside  after ");
     }
 
     #[test]
@@ -378,7 +403,7 @@ mod tests {
         formatter.write_str("second").unwrap();
         formatter.remove_label().unwrap();
         formatter.write_str("after").unwrap();
-        insta::assert_snapshot!(String::from_utf8(output).unwrap(), @"beforefirstsecondafter");
+        insta::assert_snapshot!(String::from_utf8(output).unwrap(), @"beforefirstsecondafter");
     }
 
     #[test]
@@ -397,7 +422,7 @@ mod tests {
             .unwrap();
         formatter.remove_label().unwrap();
         // TODO: Replace the ANSI escape (\x1b) by something else (🌈?)
-        insta::assert_snapshot!(String::from_utf8(output).unwrap(), @"not actually bold");
+        insta::assert_snapshot!(String::from_utf8(output).unwrap(), @"not actually bold");
     }
 
     #[test]
@@ -424,7 +449,7 @@ mod tests {
         formatter.remove_label().unwrap();
         formatter.write_str(" after outer ").unwrap();
         insta::assert_snapshot!(String::from_utf8(output).unwrap(),
-        @" before outer  before inner  inside inner  after inner  after outer ");
+        @" before outer  before inner  inside inner  after inner  after outer ");
     }
 
     #[test]
@@ -445,7 +470,7 @@ mod tests {
         formatter.write_str(" not colored ").unwrap();
         formatter.remove_label().unwrap();
         insta::assert_snapshot!(String::from_utf8(output).unwrap(),
-        @" not colored  colored  not colored ");
+        @" not colored  colored  not colored ");
     }
 
     #[test]
@@ -468,7 +493,7 @@ mod tests {
         formatter.remove_label().unwrap();
         // TODO: Make this not reset the color inside
         insta::assert_snapshot!(String::from_utf8(output).unwrap(),
-        @" red before  still red inside  also red afterwards ");
+        @" red before  still red inside  also red afterwards ");
     }
 
     #[test]
@@ -488,7 +513,7 @@ mod tests {
         formatter.remove_label().unwrap();
         formatter.remove_label().unwrap();
         insta::assert_snapshot!(String::from_utf8(output).unwrap(),
-        @" hello ");
+        @" hello ");
     }
 
     #[test]
@@ -506,8 +531,7 @@ mod tests {
         formatter.write_str(" hello ").unwrap();
         formatter.remove_label().unwrap();
         formatter.remove_label().unwrap();
-        insta::assert_snapshot!(String::from_utf8(output).unwrap(), 
+        insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" hello ");
-        @" hello ");
     }
 
     #[test]
@@ -538,7 +562,7 @@ mod tests {
         formatter.write_str(" a2 ").unwrap();
         formatter.remove_label().unwrap();
         insta::assert_snapshot!(String::from_utf8(output).unwrap(),
-        @" a1  b1  c1  d  c2  b2  a2 ");
+        @" a1  b1  c1  d  c2  b2  a2 ");
     }
 
     #[test]
@@ -567,6 +591,6 @@ mod tests {
         formatter.remove_label().unwrap();
         // TODO: This is currently not deterministic.
         // insta::assert_snapshot!(String::from_utf8(output).unwrap(),
-        // @" a1  b1  c  b2  a2 ");
+        // @" a1  b1  c  b2  a2 ");
     }
 }

tests/test_alias.rs

@@ -214,7 +214,7 @@ fn test_alias_global_args_in_definition() {
     // The global argument in the alias is respected
     let stdout = test_env.jj_cmd_success(&repo_path, &["l"]);
     insta::assert_snapshot!(stdout, @r###"
-    o 0000000000000000000000000000000000000000
+    o 0000000000000000000000000000000000000000
     "###);
 }
 

tests/test_commit_template.rs

@@ -75,22 +75,22 @@ fn test_log_default() {
     // Color
     let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--color=always"]);
     insta::assert_snapshot!(stdout, @r###"
-    @ ffdaa62087a2 test.user@example.com 2001-02-03 04:05:09.000 +07:00 my-branch 9de54178d59d
+    @ ffdaa62087a2 test.user@example.com 2001-02-03 04:05:09.000 +07:00 my-branch 9de54178d59d
-    | description 1
+    | description 1
-    o 9a45c67d3e96 test.user@example.com 2001-02-03 04:05:08.000 +07:00 4291e264ae97
+    o 9a45c67d3e96 test.user@example.com 2001-02-03 04:05:08.000 +07:00 4291e264ae97
     | add a file
-    o 000000000000  1970-01-01 00:00:00.000 +00:00 000000000000
+    o 000000000000  1970-01-01 00:00:00.000 +00:00 000000000000
       (no description set)
     "###);
 
     // Color without graph
     let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--color=always", "--no-graph"]);
     insta::assert_snapshot!(stdout, @r###"
-    ffdaa62087a2 test.user@example.com 2001-02-03 04:05:09.000 +07:00 my-branch 9de54178d59d
+    ffdaa62087a2 test.user@example.com 2001-02-03 04:05:09.000 +07:00 my-branch 9de54178d59d
-    description 1
+    description 1
-    9a45c67d3e96 test.user@example.com 2001-02-03 04:05:08.000 +07:00 4291e264ae97
+    9a45c67d3e96 test.user@example.com 2001-02-03 04:05:08.000 +07:00 4291e264ae97
     add a file
-    000000000000  1970-01-01 00:00:00.000 +00:00 000000000000
+    000000000000  1970-01-01 00:00:00.000 +00:00 000000000000
     (no description set)
     "###);
 }
@@ -131,11 +131,11 @@ fn test_log_default_divergence() {
     // Color
     let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--color=always"]);
     insta::assert_snapshot!(stdout, @r###"
-    o 9a45c67d3e96?? test.user@example.com 2001-02-03 04:05:10.000 +07:00 8979953d4c67
+    o 9a45c67d3e96?? test.user@example.com 2001-02-03 04:05:10.000 +07:00 8979953d4c67
     | description 2
-    | @ 9a45c67d3e96?? test.user@example.com 2001-02-03 04:05:08.000 +07:00 7a17d52e633c
+    | @ 9a45c67d3e96?? test.user@example.com 2001-02-03 04:05:08.000 +07:00 7a17d52e633c
-    |/  description 1
+    |/  description 1
-    o 000000000000  1970-01-01 00:00:00.000 +00:00 000000000000
+    o 000000000000  1970-01-01 00:00:00.000 +00:00 000000000000
       (no description set)
     "###);
 }

tests/test_global_opts.rs

@@ -182,8 +182,8 @@ fn test_color_config() {
     // Test that --color=always is respected.
     let stdout = test_env.jj_cmd_success(&repo_path, &["--color=always", "log", "-T", "commit_id"]);
     insta::assert_snapshot!(stdout, @r###"
-    @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22
+    @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22
-    o 0000000000000000000000000000000000000000
+    o 0000000000000000000000000000000000000000
     "###);
 
     // Test that color is used if it's requested in the config file
@@ -193,8 +193,8 @@ color="always""#,
     );
     let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "commit_id"]);
     insta::assert_snapshot!(stdout, @r###"
-    @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22
+    @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22
-    o 0000000000000000000000000000000000000000
+    o 0000000000000000000000000000000000000000
     "###);
 
     // Test that --color=never overrides the config.
@@ -249,8 +249,8 @@ color="always""#,
     test_env.add_env_var("NO_COLOR", "");
     let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "commit_id"]);
     insta::assert_snapshot!(stdout, @r###"
-    @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22
+    @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22
-    o 0000000000000000000000000000000000000000
+    o 0000000000000000000000000000000000000000
     "###);
 
     // Test that per-repo config overrides the user config.

tests/test_log_command.rs

@@ -573,13 +573,13 @@ fn test_graph_template_color() {
     // TODO: The color codes shouldn't span the graph lines, and we shouldn't get an
     // extra line at the end
     insta::assert_snapshot!(stdout, @r###"
-    @ single line
+    @ single line
-    | 
+    | 
-    o first line
+    o first line
     | second line
     | third line
-    | 
+    | 
-    o (no description set)
+    o (no description set)
-      
+      
     "###);
 }

tests/test_resolve_command.rs

@@ -386,7 +386,7 @@ fn test_too_many_parents() {
     // Test warning color
     insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list", "--color=always"]), 
     @r###"
-    file    3-sided conflict
+    file    3-sided conflict
     "###);
 
     let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]);
@@ -518,7 +518,7 @@ fn test_description_with_dir_and_deletion() {
     // Test warning color. The deletion is fine, so it's not highlighted
     insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list", "--color=always"]), 
     @r###"
-    file    3-sided conflict including 1 deletion and a directory
+    file    3-sided conflict including 1 deletion and a directory
     "###);
     let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]);
     insta::assert_snapshot!(error, @r###"
@@ -618,8 +618,8 @@ fn test_multiple_conflicts() {
     // Test colors
     insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list", "--color=always"]), 
     @r###"
-    another_file                        2-sided conflict
+    another_file                        2-sided conflict
-    this_file_has_a_very_long_name_to_test_padding 2-sided conflict
+    this_file_has_a_very_long_name_to_test_padding 2-sided conflict
     "###);
 
     let editor_script = test_env.set_up_fake_editor();