From 6ae671960c9d8dad438dbfd7f117adc8c054ee59 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Fri, 13 Jan 2023 00:39:22 -0800
Subject: [PATCH] formatter: don't write escape codes until we write text

We often end up writing escape codes for one style and then
immediately after, we write escape codes for another style. That seems
harmless, but it's a little ugly. More importantly, it prepares for
not emitting any escapes for turning off attributes at the end of
formatted contents across multiple lines (see next commit).
---
 src/formatter.rs              | 10 +++++++---
 tests/test_commit_template.rs |  6 +++---
 tests/test_global_opts.rs     |  6 +++---
 tests/test_log_command.rs     |  2 +-
 tests/test_resolve_command.rs |  4 ++--
 5 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/src/formatter.rs b/src/formatter.rs
index ccd6ab145..6f96afd2c 100644
--- a/src/formatter.rs
+++ b/src/formatter.rs
@@ -346,6 +346,7 @@ fn color_for_name(color_name: &str) -> Option<Color> {
 
 impl<W: Write> Write for ColorFormatter<W> {
     fn write(&mut self, data: &[u8]) -> Result<usize, Error> {
+        self.write_new_style()?;
         self.output.write(data)
     }
 
@@ -357,12 +358,15 @@ impl<W: Write> Write for ColorFormatter<W> {
 impl<W: Write> Formatter for ColorFormatter<W> {
     fn push_label(&mut self, label: &str) -> io::Result<()> {
         self.labels.push(label.to_owned());
-        self.write_new_style()
+        Ok(())
     }
 
     fn pop_label(&mut self) -> io::Result<()> {
         self.labels.pop();
-        self.write_new_style()
+        if self.labels.is_empty() {
+            self.write_new_style()?
+        }
+        Ok(())
     }
 }
 
@@ -511,7 +515,7 @@ mod tests {
          bold only 
          underlined only 
          single rule 
-         two rules 
+         two rules 
         "###);
     }
 
diff --git a/tests/test_commit_template.rs b/tests/test_commit_template.rs
index caceca342..12b434199 100644
--- a/tests/test_commit_template.rs
+++ b/tests/test_commit_template.rs
@@ -80,7 +80,7 @@ fn test_log_default() {
     | 
     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)
     "###);
 
@@ -91,7 +91,7 @@ fn test_log_default() {
     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
+    000000000000  1970-01-01 00:00:00.000 +00:00 000000000000
     (no description set)
     "###);
 }
@@ -138,7 +138,7 @@ fn test_log_default_divergence() {
     | @ 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
+    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 fca08b60d..3d2a75262 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 ffc19aeaf..e9b51cfb2 100644
--- a/tests/test_log_command.rs
+++ b/tests/test_log_command.rs
@@ -574,7 +574,7 @@ fn test_graph_template_color() {
     // extra line at the end
     insta::assert_snapshot!(stdout, @r###"
     @ single line
-    | 
+    | 
     o first line
     | second line
     | third line
diff --git a/tests/test_resolve_command.rs b/tests/test_resolve_command.rs
index 8420db282..6092096fc 100644
--- a/tests/test_resolve_command.rs
+++ b/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###"