From 2a9c37a693b2226dd2d4da0693ea7f1e57fdabd6 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Sat, 7 Jan 2023 10:16:57 -0800
Subject: [PATCH] formatter: remove hack for making bright colors bold

We can now configure the working-copy commit and the head operation to
use bold font, so we no longer need the hack to make bright colors
bold.
---
 src/config/colors.toml        |  7 ++-----
 src/formatter.rs              | 39 +++++++----------------------------
 tests/test_commit_template.rs | 17 ++++++++-------
 tests/test_global_opts.rs     |  6 +++---
 tests/test_log_command.rs     |  4 ++--
 5 files changed, 25 insertions(+), 48 deletions(-)

diff --git a/src/config/colors.toml b/src/config/colors.toml
index e0f082814..53d7b66b9 100644
--- a/src/config/colors.toml
+++ b/src/config/colors.toml
@@ -20,11 +20,7 @@
 "divergent change_id"="red"
 "conflict" = "red"
 
-# TODO: This near-duplication of the lines above is unfortunate. Should we
-# allow adding and clearing the "bright" bit somehow? Or should we instead
-# use a different background color? (We don't have support for background
-# colors yet.)
-
+"working_copy" = { bold = true }
 "working_copy commit_id" = "bright blue"
 "working_copy change_id" = "bright magenta"
 "working_copy email" = "bright yellow"
@@ -48,6 +44,7 @@
 "op-log user" = "yellow"
 "op-log time" = "cyan"
 "op-log tags" = "white"
+"op-log head" = { bold = true }
 "op-log head id" = "bright blue"
 "op-log head user" = "bright yellow"
 "op-log head time" = "bright cyan"
diff --git a/src/formatter.rs b/src/formatter.rs
index 32c7ac16e..d17bb72ee 100644
--- a/src/formatter.rs
+++ b/src/formatter.rs
@@ -231,15 +231,6 @@ impl<W: Write> ColorFormatter<W> {
     fn write_new_style(&mut self) -> io::Result<()> {
         let new_style = self.current_style();
         if new_style != self.current_style {
-            // 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_style.fg_color) && is_bright(&new_style.fg_color) {
-                queue!(self.output, SetAttribute(Attribute::Bold))?;
-            } else if !is_bright(&new_style.fg_color) && is_bright(&self.current_style.fg_color) {
-                queue!(self.output, SetAttribute(Attribute::Reset))?;
-            }
             if new_style.bold != self.current_style.bold {
                 if new_style.bold.unwrap_or_default() {
                     queue!(self.output, SetAttribute(Attribute::Bold))?;
@@ -270,20 +261,6 @@ impl<W: Write> ColorFormatter<W> {
     }
 }
 
-fn is_bright(color: &Option<Color>) -> bool {
-    matches!(
-        color.unwrap_or(Color::Reset),
-        Color::DarkGrey
-            | Color::Red
-            | Color::Green
-            | Color::Yellow
-            | Color::Blue
-            | Color::Magenta
-            | Color::Cyan
-            | Color::White
-    )
-}
-
 fn rules_from_config(config: &config::Config) -> HashMap<Vec<String>, Style> {
     let mut result = HashMap::new();
     if let Ok(table) = config.get_table("colors") {
@@ -439,14 +416,14 @@ mod tests {
          magenta 
          cyan 
          white 
-         bright black 
-         bright red 
-         bright green 
-         bright yellow 
-         bright blue 
-         bright magenta 
-         bright cyan 
-         bright white 
+         bright black 
+         bright red 
+         bright green 
+         bright yellow 
+         bright blue 
+         bright magenta 
+         bright cyan 
+         bright white 
         "###);
     }
 
diff --git a/tests/test_commit_template.rs b/tests/test_commit_template.rs
index 509585ea1..caceca342 100644
--- a/tests/test_commit_template.rs
+++ b/tests/test_commit_template.rs
@@ -75,8 +75,9 @@ 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
-    | description 1
+    @ ffdaa62087a2 test.user@example.com 2001-02-03 04:05:09.000 +07:00 my-branch 9de54178d59d
+    | description 1
+    | 
     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
@@ -86,9 +87,9 @@ fn test_log_default() {
     // 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
-    description 1
-    9a45c67d3e96 test.user@example.com 2001-02-03 04:05:08.000 +07:00 4291e264ae97
+    ffdaa62087a2 test.user@example.com 2001-02-03 04:05:09.000 +07:00 my-branch 9de54178d59d
+    description 1
+    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
     (no description set)
@@ -129,12 +130,14 @@ fn test_log_default_divergence() {
     "###);
 
     // Color
+    // TODO: We shouldn't get a blank line after "description 1"
     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
     | description 2
-    | @ 9a45c67d3e96?? test.user@example.com 2001-02-03 04:05:08.000 +07:00 7a17d52e633c
-    |/  description 1
+    | @ 9a45c67d3e96?? test.user@example.com 2001-02-03 04:05:08.000 +07:00 7a17d52e633c
+    |/  description 1
+    |   
     o 000000000000  1970-01-01 00:00:00.000 +00:00 000000000000
       (no description set)
     "###);
diff --git a/tests/test_global_opts.rs b/tests/test_global_opts.rs
index 58755bde0..fca08b60d 100644
--- a/tests/test_global_opts.rs
+++ b/tests/test_global_opts.rs
@@ -182,7 +182,7 @@ 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
     "###);
 
@@ -193,7 +193,7 @@ color="always""#,
     );
     let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "commit_id"]);
     insta::assert_snapshot!(stdout, @r###"
-    @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22
+    @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22
     o 0000000000000000000000000000000000000000
     "###);
 
@@ -249,7 +249,7 @@ 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
     "###);
 
diff --git a/tests/test_log_command.rs b/tests/test_log_command.rs
index b6d33ca27..ffc19aeaf 100644
--- a/tests/test_log_command.rs
+++ b/tests/test_log_command.rs
@@ -573,8 +573,8 @@ 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
     | second line
     | third line