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).
This commit is contained in:
Martin von Zweigbergk 2023-01-13 00:39:22 -08:00 committed by Martin von Zweigbergk
parent 041b42ced6
commit 6ae671960c
5 changed files with 16 additions and 12 deletions

View file

@ -346,6 +346,7 @@ fn color_for_name(color_name: &str) -> Option<Color> {
impl<W: Write> Write for ColorFormatter<W> { impl<W: Write> Write for ColorFormatter<W> {
fn write(&mut self, data: &[u8]) -> Result<usize, Error> { fn write(&mut self, data: &[u8]) -> Result<usize, Error> {
self.write_new_style()?;
self.output.write(data) self.output.write(data)
} }
@ -357,12 +358,15 @@ impl<W: Write> Write for ColorFormatter<W> {
impl<W: Write> Formatter for ColorFormatter<W> { impl<W: Write> Formatter for ColorFormatter<W> {
fn push_label(&mut self, label: &str) -> io::Result<()> { fn push_label(&mut self, label: &str) -> io::Result<()> {
self.labels.push(label.to_owned()); self.labels.push(label.to_owned());
self.write_new_style() Ok(())
} }
fn pop_label(&mut self) -> io::Result<()> { fn pop_label(&mut self) -> io::Result<()> {
self.labels.pop(); 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   bold only 
 underlined only   underlined only 
 single rule   single rule 
 two rules   two rules 
"###); "###);
} }

View file

@ -80,7 +80,7 @@ fn test_log_default() {
|  | 
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 | 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) (no description set)
"###); "###);
@ -91,7 +91,7 @@ fn test_log_default() {
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 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) (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 | @ 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) (no description set)
"###); "###);
} }

View file

@ -182,7 +182,7 @@ fn test_color_config() {
// Test that --color=always is respected. // Test that --color=always is respected.
let stdout = test_env.jj_cmd_success(&repo_path, &["--color=always", "log", "-T", "commit_id"]); let stdout = test_env.jj_cmd_success(&repo_path, &["--color=always", "log", "-T", "commit_id"]);
insta::assert_snapshot!(stdout, @r###" insta::assert_snapshot!(stdout, @r###"
@ 230dd059e1b059aefc0da06a2e5a7dbf22362f22 @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22
o 0000000000000000000000000000000000000000 o 0000000000000000000000000000000000000000
"###); "###);
@ -193,7 +193,7 @@ color="always""#,
); );
let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "commit_id"]); let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "commit_id"]);
insta::assert_snapshot!(stdout, @r###" insta::assert_snapshot!(stdout, @r###"
@ 230dd059e1b059aefc0da06a2e5a7dbf22362f22 @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22
o 0000000000000000000000000000000000000000 o 0000000000000000000000000000000000000000
"###); "###);
@ -249,7 +249,7 @@ color="always""#,
test_env.add_env_var("NO_COLOR", ""); test_env.add_env_var("NO_COLOR", "");
let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "commit_id"]); let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "commit_id"]);
insta::assert_snapshot!(stdout, @r###" insta::assert_snapshot!(stdout, @r###"
@ 230dd059e1b059aefc0da06a2e5a7dbf22362f22 @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22
o 0000000000000000000000000000000000000000 o 0000000000000000000000000000000000000000
"###); "###);

View file

@ -574,7 +574,7 @@ fn test_graph_template_color() {
// extra line at the end // extra line at the end
insta::assert_snapshot!(stdout, @r###" insta::assert_snapshot!(stdout, @r###"
@ single line @ single line
|  | 
o first line o first line
| second line | second line
| third line | third line

View file

@ -386,7 +386,7 @@ fn test_too_many_parents() {
// Test warning color // Test warning color
insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list", "--color=always"]), insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list", "--color=always"]),
@r###" @r###"
file 3-sided conflict file 3-sided conflict
"###); "###);
let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]); 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 // 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"]), insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["resolve", "--list", "--color=always"]),
@r###" @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"]); let error = test_env.jj_cmd_failure(&repo_path, &["resolve"]);
insta::assert_snapshot!(error, @r###" insta::assert_snapshot!(error, @r###"