just playing with tangled
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

formatter: reset color around newlines

There are several reasons for this:

* We can more easily skip styling a trailing blank line, which other
internal code then can correctly detect as having a trailing
newline. This fixes the TODO in tests/test_commit_template.rs.

* Some tools (like `less -R`) add an extra newline if the final
character is not a newline (e.g. if there's a color reset after
it), which led to an annoying blank line after the diff summary in
e.g. `jj status`.

* Since each line is styled independently, you get all the necessary
escapes even when grepping through the output.

* Some terminals extend background color to the end of the terminal
(i.e. past the newline character), which is probably not what the
user wanted.

* Some tools (like `less -R`) get confused and lose coloring of lines
after a newline.

authored by

Martin von Zweigbergk and committed by
Martin von Zweigbergk
31ad0cd2 6ae67196

+50 -23
+38 -3
src/formatter.rs
··· 16 16 use std::collections::HashMap; 17 17 use std::io::{Error, Write}; 18 18 use std::sync::Arc; 19 - use std::{fmt, io}; 19 + use std::{fmt, io, mem}; 20 20 21 21 use crossterm::queue; 22 22 use crossterm::style::{Attribute, Color, SetAttribute, SetBackgroundColor, SetForegroundColor}; ··· 346 346 347 347 impl<W: Write> Write for ColorFormatter<W> { 348 348 fn write(&mut self, data: &[u8]) -> Result<usize, Error> { 349 - self.write_new_style()?; 350 - self.output.write(data) 349 + /* 350 + We clear the current style at the end of each line, and then we re-apply the style 351 + after the newline. There are several reasons for this: 352 + 353 + * We can more easily skip styling a trailing blank line, which other 354 + internal code then can correctly detect as having a trailing 355 + newline. 356 + 357 + * Some tools (like `less -R`) add an extra newline if the final 358 + character is not a newline (e.g. if there's a color reset after 359 + it), which led to an annoying blank line after the diff summary in 360 + e.g. `jj status`. 361 + 362 + * Since each line is styled independently, you get all the necessary 363 + escapes even when grepping through the output. 364 + 365 + * Some terminals extend background color to the end of the terminal 366 + (i.e. past the newline character), which is probably not what the 367 + user wanted. 368 + 369 + * Some tools (like `less -R`) get confused and lose coloring of lines 370 + after a newline. 371 + */ 372 + for line in data.split_inclusive(|b| *b == b'\n') { 373 + if line.ends_with(b"\n") { 374 + self.write_new_style()?; 375 + self.output.write_all(&line[..line.len() - 1])?; 376 + let labels = mem::take(&mut self.labels); 377 + self.write_new_style()?; 378 + self.output.write_all(b"\n")?; 379 + self.labels = labels; 380 + } else { 381 + self.write_new_style()?; 382 + self.output.write_all(line)?; 383 + } 384 + } 385 + Ok(data.len()) 351 386 } 352 387 353 388 fn flush(&mut self) -> Result<(), Error> {
+7 -10
tests/test_commit_template.rs
··· 75 75 // Color 76 76 let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--color=always"]); 77 77 insta::assert_snapshot!(stdout, @r###" 78 - @ ffdaa62087a2 test.user@example.com 2001-02-03 04:05:09.000 +07:00 my-branch 9de54178d59d 79 - | description 1 80 - |  78 + @ ffdaa62087a2 test.user@example.com 2001-02-03 04:05:09.000 +07:00 my-branch 9de54178d59d 79 + | description 1 81 80 o 9a45c67d3e96 test.user@example.com 2001-02-03 04:05:08.000 +07:00 4291e264ae97 82 81 | add a file 83 82 o 000000000000 1970-01-01 00:00:00.000 +00:00 000000000000 ··· 87 86 // Color without graph 88 87 let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--color=always", "--no-graph"]); 89 88 insta::assert_snapshot!(stdout, @r###" 90 - ffdaa62087a2 test.user@example.com 2001-02-03 04:05:09.000 +07:00 my-branch 9de54178d59d 91 - description 1 92 - 9a45c67d3e96 test.user@example.com 2001-02-03 04:05:08.000 +07:00 4291e264ae97 89 + ffdaa62087a2 test.user@example.com 2001-02-03 04:05:09.000 +07:00 my-branch 9de54178d59d 90 + description 1 91 + 9a45c67d3e96 test.user@example.com 2001-02-03 04:05:08.000 +07:00 4291e264ae97 93 92 add a file 94 93 000000000000 1970-01-01 00:00:00.000 +00:00 000000000000 95 94 (no description set) ··· 130 129 "###); 131 130 132 131 // Color 133 - // TODO: We shouldn't get a blank line after "description 1" 134 132 let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--color=always"]); 135 133 insta::assert_snapshot!(stdout, @r###" 136 134 o 9a45c67d3e96?? test.user@example.com 2001-02-03 04:05:10.000 +07:00 8979953d4c67 137 135 | description 2 138 - | @ 9a45c67d3e96?? test.user@example.com 2001-02-03 04:05:08.000 +07:00 7a17d52e633c 139 - |/ description 1 140 - |  136 + | @ 9a45c67d3e96?? test.user@example.com 2001-02-03 04:05:08.000 +07:00 7a17d52e633c 137 + |/ description 1 141 138 o 000000000000 1970-01-01 00:00:00.000 +00:00 000000000000 142 139 (no description set) 143 140 "###);
+5 -10
tests/test_log_command.rs
··· 570 570 o (no description set) 571 571 "###); 572 572 let stdout = test_env.jj_cmd_success(&repo_path, &["--color=always", "log", "-T=description"]); 573 - // TODO: The color codes shouldn't span the graph lines, and we shouldn't get an 574 - // extra line at the end 575 573 insta::assert_snapshot!(stdout, @r###" 576 - @ single line 577 - |  578 - o first line 579 - | second line 580 - | third line 581 - |  582 - o (no description set) 583 -  574 + @ single line 575 + o first line 576 + | second line 577 + | third line 578 + o (no description set) 584 579 "###); 585 580 }