Skip to content

Commit

Permalink
--color=debug: combine segments with same labels
Browse files Browse the repository at this point in the history 
This not only makes the output easier to read, but also protects against
implementation detail changes in `write!` when used with a format
string (especially, how many times and with what strings it calls the
underlying writer).
  • Loading branch information
@jonathantanmy
jonathantanmy committed Jul 11, 2024
1 parent 8ea1a68 commit 2b8383b
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 61 deletions.
52 changes: 26 additions & 26 deletions cli/src/formatter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,25 +264,27 @@ impl Style {
#[derive(Clone, Debug)]
pub struct ColorFormatter<W: Write> {
output: W,
debug: bool,
rules: Arc<Rules>,
/// The stack of currently applied labels. These determine the desired
/// style.
labels: Vec<String>,
cached_styles: HashMap<Vec<String>, Style>,
/// The style we last wrote to the output.
current_style: Style,
/// The debug string (space-separated labels) we last wrote to the output.
/// Initialize to None to turn debug strings off.
current_debug: Option<String>,
}

impl<W: Write> ColorFormatter<W> {
pub fn new(output: W, rules: Arc<Rules>, debug: bool) -> ColorFormatter<W> {
ColorFormatter {
output,
rules,
debug,
labels: vec![],
cached_styles: HashMap::new(),
current_style: Style::default(),
current_debug: if debug { Some(String::new()) } else { None },
}
}

Expand Down Expand Up @@ -335,6 +337,20 @@ impl<W: Write> ColorFormatter<W> {
}

fn write_new_style(&mut self) -> io::Result<()> {
let new_debug = match &self.current_debug {
Some(current) => {
let joined = self.labels.join(" ");
if joined.eq(current) {
None
} else {
if !current.is_empty() {
write!(self.output, ">>")?;
}
Some(joined)
}
}
None => None,
};
let new_style = self.requested_style();
if new_style != self.current_style {
if new_style.bold != self.current_style.bold {
Expand Down Expand Up @@ -370,6 +386,12 @@ impl<W: Write> ColorFormatter<W> {
}
self.current_style = new_style;
}
if let Some(d) = new_debug {
if !d.is_empty() {
write!(self.output, "<<{}::", d)?;
}
self.current_debug = Some(d);
}
Ok(())
}
}
Expand Down Expand Up @@ -492,39 +514,17 @@ impl<W: Write> Write for ColorFormatter<W> {
for line in data.split_inclusive(|b| *b == b'\n') {
if line.ends_with(b"\n") {
self.write_new_style()?;
write_line_exclusive(
&mut self.output,
&line[..line.len() - 1],
&self.labels,
self.debug,
)?;
write_sanitized(&mut self.output, &line[..line.len() - 1])?;
let labels = mem::take(&mut self.labels);
self.write_new_style()?;
self.output.write_all(b"\n")?;
self.labels = labels;
} else {
self.write_new_style()?;
write_line_exclusive(&mut self.output, line, &self.labels, self.debug)?;
write_sanitized(&mut self.output, line)?;
}
}

fn write_line_exclusive<W: Write>(
output: &mut W,
buf: &[u8],
labels: &[String],
debug: bool,
) -> io::Result<()> {
if debug && !labels.is_empty() {
write!(output, "<<{}::", labels.join(" "))?;
write_sanitized(output, buf)?;
write!(output, ">>")?;
} else {
write_sanitized(output, buf)?;
}

Ok(())
}

Ok(data.len())
}

Expand Down
18 changes: 9 additions & 9 deletions cli/tests/test_commit_template.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,24 +407,24 @@ fn test_log_builtin_templates_colored_debug() {
<<node::@>> <<log::Commit ID: >><<log commit_id::dc31539712c7294d1d712cec63cef4504b94ca74>><<log::>>
│ <<log::Change ID: >><<log change_id::rlvkpnrzqnoowoytxnquwvuryrwnrmlp>><<log::>>
│ <<log::Branches: >><<log local_branches name::my-branch>><<log::>>
│ <<log::Author: >>[38;5;1m<<log name placeholder::(no name set)>>[39m<<log:: <>>[38;5;1m<<log email placeholder::(no email set)>>[39m<<log::>>><<log:: (>>[38;5;6m<<log author timestamp local format::2001-02-03 08:05:08>>[39m<<log::)>><<log::>>
│ <<log::Committer: >>[38;5;1m<<log name placeholder::(no name set)>>[39m<<log:: <>>[38;5;1m<<log email placeholder::(no email set)>>[39m<<log::>>><<log:: (>>[38;5;6m<<log committer timestamp local format::2001-02-03 08:05:08>>[39m<<log::)>><<log::>>
│ <<log::Author: >>[38;5;1m<<log name placeholder::(no name set)>>[39m<<log:: <>>[38;5;1m<<log email placeholder::(no email set)>>[39m<<log::> (>>[38;5;6m<<log author timestamp local format::2001-02-03 08:05:08>>[39m<<log::)>>
│ <<log::Committer: >>[38;5;1m<<log name placeholder::(no name set)>>[39m<<log:: <>>[38;5;1m<<log email placeholder::(no email set)>>[39m<<log::> (>>[38;5;6m<<log committer timestamp local format::2001-02-03 08:05:08>>[39m<<log::)>>
│ <<log::>>
│ [38;5;2m<<log empty description placeholder:: >><<log empty description placeholder::(no description set)>>[39m<<log::>>
│ [38;5;2m<<log empty description placeholder:: (no description set)>>[39m<<log::>>
│ <<log::>>
<<node::◉>> <<log::Commit ID: >><<log commit_id::230dd059e1b059aefc0da06a2e5a7dbf22362f22>><<log::>>
│ <<log::Change ID: >><<log change_id::qpvuntsmwlqtpsluzzsnyyzlmlwvmlnu>><<log::>>
│ <<log::Author: >><<log author name::Test User>><<log:: <>>[38;5;3m<<log author email::[email protected]>>[39m<<log::>>><<log:: (>>[38;5;6m<<log author timestamp local format::2001-02-03 08:05:07>>[39m<<log::)>><<log::>>
│ <<log::Committer: >><<log committer name::Test User>><<log:: <>>[38;5;3m<<log committer email::[email protected]>>[39m<<log::>>><<log:: (>>[38;5;6m<<log committer timestamp local format::2001-02-03 08:05:07>>[39m<<log::)>><<log::>>
│ <<log::Author: >><<log author name::Test User>><<log:: <>>[38;5;3m<<log author email::[email protected]>>[39m<<log::> (>>[38;5;6m<<log author timestamp local format::2001-02-03 08:05:07>>[39m<<log::)>>
│ <<log::Committer: >><<log committer name::Test User>><<log:: <>>[38;5;3m<<log committer email::[email protected]>>[39m<<log::> (>>[38;5;6m<<log committer timestamp local format::2001-02-03 08:05:07>>[39m<<log::)>>
│ <<log::>>
│ [38;5;2m<<log empty description placeholder:: >><<log empty description placeholder::(no description set)>>[39m<<log::>>
│ [38;5;2m<<log empty description placeholder:: (no description set)>>[39m<<log::>>
│ <<log::>>
<<node::◉>> <<log::Commit ID: >><<log commit_id::0000000000000000000000000000000000000000>><<log::>>
<<log::Change ID: >><<log change_id::zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz>><<log::>>
<<log::Author: >>[38;5;1m<<log name placeholder::(no name set)>>[39m<<log:: <>>[38;5;1m<<log email placeholder::(no email set)>>[39m<<log::>>><<log:: (>>[38;5;6m<<log author timestamp local format::1970-01-01 11:00:00>>[39m<<log::)>><<log::>>
<<log::Committer: >>[38;5;1m<<log name placeholder::(no name set)>>[39m<<log:: <>>[38;5;1m<<log email placeholder::(no email set)>>[39m<<log::>>><<log:: (>>[38;5;6m<<log committer timestamp local format::1970-01-01 11:00:00>>[39m<<log::)>><<log::>>
<<log::Author: >>[38;5;1m<<log name placeholder::(no name set)>>[39m<<log:: <>>[38;5;1m<<log email placeholder::(no email set)>>[39m<<log::> (>>[38;5;6m<<log author timestamp local format::1970-01-01 11:00:00>>[39m<<log::)>>
<<log::Committer: >>[38;5;1m<<log name placeholder::(no name set)>>[39m<<log:: <>>[38;5;1m<<log email placeholder::(no email set)>>[39m<<log::> (>>[38;5;6m<<log committer timestamp local format::1970-01-01 11:00:00>>[39m<<log::)>>
<<log::>>
[38;5;2m<<log empty description placeholder:: >><<log empty description placeholder::(no description set)>>[39m<<log::>>
[38;5;2m<<log empty description placeholder:: (no description set)>>[39m<<log::>>
<<log::>>
"###);
}
Expand Down
52 changes: 26 additions & 26 deletions cli/tests/test_diff_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,14 @@ fn test_diff_basic() {

let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--color=debug"]);
insta::assert_snapshot!(stdout, @r###"
[38;5;3m<<diff header::Removed >><<diff header::regular file>><<diff header:: >><<diff header::file1>><<diff header:::>>[39m
[38;5;1m<<diff removed line_number:: >><<diff removed line_number:: >><<diff removed line_number:: >><<diff removed line_number::1>>[39m<<diff:: >><<diff:: : >>[4m[38;5;1m<<diff removed token::foo>>[24m[39m
[38;5;3m<<diff header::Modified regular file>><<diff header:: >><<diff header::file2>><<diff header:::>>[39m
[38;5;1m<<diff removed line_number:: >><<diff removed line_number:: >><<diff removed line_number:: >><<diff removed line_number::1>>[39m<<diff:: >>[38;5;2m<<diff added line_number:: >><<diff added line_number:: >><<diff added line_number:: >><<diff added line_number::1>>[39m<<diff::: >><<diff::foo>>
<<diff:: >>[38;5;2m<<diff added line_number:: >><<diff added line_number:: >><<diff added line_number:: >><<diff added line_number::2>>[39m<<diff::: >>[4m[38;5;2m<<diff added token::bar>>[24m[39m
[38;5;1m<<diff removed line_number:: >><<diff removed line_number:: >><<diff removed line_number:: >><<diff removed line_number::2>>[39m<<diff:: >>[38;5;2m<<diff added line_number:: >><<diff added line_number:: >><<diff added line_number:: >><<diff added line_number::3>>[39m<<diff::: >><<diff::baz >>[4m[38;5;1m<<diff removed token::qux>>[38;5;2m<<diff added token::quux>>[24m[39m<<diff::>>
[38;5;3m<<diff header::Added >><<diff header::regular file>><<diff header:: >><<diff header::file3>><<diff header:::>>[39m
<<diff:: >>[38;5;2m<<diff added line_number:: >><<diff added line_number:: >><<diff added line_number:: >><<diff added line_number::1>>[39m<<diff::: >>[4m[38;5;2m<<diff added token::foo>>[24m[39m
[38;5;3m<<diff header::Removed regular file file1:>>[39m
[38;5;1m<<diff removed line_number:: 1>>[39m<<diff:: : >>[4m[38;5;1m<<diff removed token::foo>>[24m[39m
[38;5;3m<<diff header::Modified regular file file2:>>[39m
[38;5;1m<<diff removed line_number:: 1>>[39m<<diff:: >>[38;5;2m<<diff added line_number:: 1>>[39m<<diff::: foo>>
<<diff:: >>[38;5;2m<<diff added line_number:: 2>>[39m<<diff::: >>[4m[38;5;2m<<diff added token::bar>>[24m[39m
[38;5;1m<<diff removed line_number:: 2>>[39m<<diff:: >>[38;5;2m<<diff added line_number:: 3>>[39m<<diff::: baz >>[4m[38;5;1m<<diff removed token::qux>>[38;5;2m<<diff added token::quux>>[24m[39m<<diff::>>
[38;5;3m<<diff header::Added regular file file3:>>[39m
<<diff:: >>[38;5;2m<<diff added line_number:: 1>>[39m<<diff::: >>[4m[38;5;2m<<diff added token::foo>>[24m[39m
"###);

let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s"]);
Expand Down Expand Up @@ -134,28 +134,28 @@ fn test_diff_basic() {

let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git", "--color=debug"]);
insta::assert_snapshot!(stdout, @r###"
[1m<<diff file_header::diff --git a/>><<diff file_header::file1>><<diff file_header:: b/>><<diff file_header::file1>><<diff file_header::>>[0m
[1m<<diff file_header::deleted file mode >><<diff file_header::100644>><<diff file_header::>>[0m
[1m<<diff file_header::index >><<diff file_header::257cc5642c>><<diff file_header::..0000000000>>[0m
[1m<<diff file_header::--- a/>><<diff file_header::file1>><<diff file_header::>>[0m
[1m<<diff file_header::diff --git a/file1 b/file1>>[0m
[1m<<diff file_header::deleted file mode 100644>>[0m
[1m<<diff file_header::index 257cc5642c..0000000000>>[0m
[1m<<diff file_header::--- a/file1>>[0m
<<diff file_header::+++ /dev/null>>
[38;5;6m<<diff hunk_header::@@ ->><<diff hunk_header::1>><<diff hunk_header::,>><<diff hunk_header::1>><<diff hunk_header:: +>><<diff hunk_header::1>><<diff hunk_header::,>><<diff hunk_header::0>><<diff hunk_header:: @@>>[39m
[38;5;6m<<diff hunk_header::@@ -1,1 +1,0 @@>>[39m
<<diff removed::->><<diff removed token::foo>>
[1m<<diff file_header::diff --git a/>><<diff file_header::file2>><<diff file_header:: b/>><<diff file_header::file2>><<diff file_header::>>[0m
[1m<<diff file_header::index >><<diff file_header::523a4a9de8>><<diff file_header::...>><<diff file_header::485b56a572>><<diff file_header:: >><<diff file_header::100644>><<diff file_header::>>[0m
[1m<<diff file_header::--- a/>><<diff file_header::file2>><<diff file_header::>>[0m
[1m<<diff file_header::+++ b/>><<diff file_header::file2>><<diff file_header::>>[0m
[38;5;6m<<diff hunk_header::@@ ->><<diff hunk_header::1>><<diff hunk_header::,>><<diff hunk_header::2>><<diff hunk_header:: +>><<diff hunk_header::1>><<diff hunk_header::,>><<diff hunk_header::3>><<diff hunk_header:: @@>>[39m
<<diff context:: >><<diff context::foo>>
[38;5;1m<<diff removed::->><<diff removed::baz >>[4m<<diff removed token::qux>>[24m<<diff removed::>>[39m
[1m<<diff file_header::diff --git a/file2 b/file2>>[0m
[1m<<diff file_header::index 523a4a9de8...485b56a572 100644>>[0m
[1m<<diff file_header::--- a/file2>>[0m
[1m<<diff file_header::+++ b/file2>>[0m
[38;5;6m<<diff hunk_header::@@ -1,2 +1,3 @@>>[39m
<<diff context:: foo>>
[38;5;1m<<diff removed::-baz >>[4m<<diff removed token::qux>>[24m<<diff removed::>>[39m
<<diff added::+>><<diff added token::bar>>
[38;5;2m<<diff added::+>><<diff added::baz >>[4m<<diff added token::quux>>[24m<<diff added::>>[39m
[1m<<diff file_header::diff --git a/>><<diff file_header::file3>><<diff file_header:: b/>><<diff file_header::file3>><<diff file_header::>>[0m
[1m<<diff file_header::new file mode >><<diff file_header::100644>><<diff file_header::>>[0m
[1m<<diff file_header::index 0000000000..>><<diff file_header::257cc5642c>><<diff file_header::>>[0m
[38;5;2m<<diff added::+baz >>[4m<<diff added token::quux>>[24m<<diff added::>>[39m
[1m<<diff file_header::diff --git a/file3 b/file3>>[0m
[1m<<diff file_header::new file mode 100644>>[0m
[1m<<diff file_header::index 0000000000..257cc5642c>>[0m
<<diff file_header::--- /dev/null>>
[1m<<diff file_header::+++ b/>><<diff file_header::file3>><<diff file_header::>>[0m
[38;5;6m<<diff hunk_header::@@ ->><<diff hunk_header::1>><<diff hunk_header::,>><<diff hunk_header::0>><<diff hunk_header:: +>><<diff hunk_header::1>><<diff hunk_header::,>><<diff hunk_header::1>><<diff hunk_header:: @@>>[39m
[1m<<diff file_header::+++ b/file3>>[0m
[38;5;6m<<diff hunk_header::@@ -1,0 +1,1 @@>>[39m
<<diff added::+>><<diff added token::foo>>
"###);

Expand Down

0 comments on commit 2b8383b

Please sign in to comment.