From 82b62e92f10b25588f1b1a97f8a292ec107c51a3 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Wed, 2 Jun 2021 21:43:00 -0700 Subject: [PATCH] cli: respect color choice in graph logs too This patch makes it so we use color in the graph iff we use it other output. We currently always use color except for in the smoke tests, so it has no effect in practice. It's easy to turn off color when stdout is redirected (using the `atty` crate), but I haven't done that because I occasionally pipe `jj log` output to `less` and I want color then. --- src/commands.rs | 25 +++++++++++-------------- src/ui.rs | 39 ++++++++++++++++++++++++++++----------- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index e2765d108..cf4c5ed48 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -59,7 +59,7 @@ use pest::Parser; use self::chrono::{FixedOffset, TimeZone, Utc}; use crate::commands::CommandError::UserError; use crate::diff_edit::DiffEditError; -use crate::formatter::{ColorFormatter, Formatter}; +use crate::formatter::Formatter; use crate::graphlog::{AsciiGraphDrawer, Edge}; use crate::template_parser::TemplateParser; use crate::templater::Template; @@ -1055,7 +1055,7 @@ fn cmd_diff( let summary = from_tree.diff_summary(&to_tree); show_diff_summary(ui, repo.working_copy_path(), &summary)?; } else { - let mut formatter = ui.formatter(); + let mut formatter = ui.stdout_formatter(); formatter.add_label(String::from("diff"))?; for (path, diff) in from_tree.diff(&to_tree) { let ui_path = ui.format_file_path(repo.working_copy_path(), &path); @@ -1296,7 +1296,7 @@ fn cmd_log( let template = crate::template_parser::parse_commit_template(repo.as_repo_ref(), &template_string); - let mut formatter = ui.formatter(); + let mut formatter = ui.stdout_formatter(); let mut formatter = formatter.as_mut(); formatter.add_label(String::from("log"))?; @@ -1327,12 +1327,11 @@ fn cmd_log( graphlog_edges.push(Edge::Missing); } let mut buffer = vec![]; - // TODO: only use color if requested { let writer = Box::new(&mut buffer); - let mut formatter = ColorFormatter::new(writer, ui.settings()); + let mut formatter = ui.new_formatter(writer); let commit = store.get_commit(&index_entry.commit_id()).unwrap(); - template.format(&commit, &mut formatter)?; + template.format(&commit, formatter.as_mut())?; } if !buffer.ends_with(b"\n") { buffer.push(b'\n'); @@ -1385,7 +1384,7 @@ fn cmd_obslog( &template_string, ); - let mut formatter = ui.formatter(); + let mut formatter = ui.stdout_formatter(); let mut formatter = formatter.as_mut(); formatter.add_label(String::from("log"))?; @@ -1402,11 +1401,10 @@ fn cmd_obslog( edges.push(Edge::direct(predecessor.id().clone())); } let mut buffer = vec![]; - // TODO: only use color if requested { let writer = Box::new(&mut buffer); - let mut formatter = ColorFormatter::new(writer, ui.settings()); - template.format(&commit, &mut formatter)?; + let mut formatter = ui.new_formatter(writer); + template.format(&commit, formatter.as_mut())?; } if !buffer.ends_with(b"\n") { buffer.push(b'\n'); @@ -2227,7 +2225,7 @@ fn cmd_op_log( let repo = repo_command.repo(); let head_op = repo.operation().clone(); let head_op_id = head_op.id().clone(); - let mut formatter = ui.formatter(); + let mut formatter = ui.stdout_formatter(); let mut formatter = formatter.as_mut(); struct OpTemplate; impl Template for OpTemplate { @@ -2276,11 +2274,10 @@ fn cmd_op_log( edges.push(Edge::direct(parent.id().clone())); } let mut buffer = vec![]; - // TODO: only use color if requested { let writer = Box::new(&mut buffer); - let mut formatter = ColorFormatter::new(writer, ui.settings()); - template.format(&op, &mut formatter)?; + let mut formatter = ui.new_formatter(writer); + template.format(&op, formatter.as_mut())?; } if !buffer.ends_with(b"\n") { buffer.push(b'\n'); diff --git a/src/ui.rs b/src/ui.rs index ae4f0617b..5f901f651 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -27,10 +27,23 @@ use crate::templater::TemplateFormatter; pub struct Ui<'a> { cwd: PathBuf, + color: bool, formatter: Mutex>, settings: UserSettings, } +fn new_formatter<'output>( + settings: &UserSettings, + color: bool, + output: Box, +) -> Box { + if color { + Box::new(ColorFormatter::new(output, &settings)) + } else { + Box::new(PlainTextFormatter::new(output)) + } +} + impl<'stdout> Ui<'stdout> { pub fn new( cwd: PathBuf, @@ -38,14 +51,11 @@ impl<'stdout> Ui<'stdout> { is_atty: bool, settings: UserSettings, ) -> Ui<'stdout> { - let formatter: Box = if is_atty { - Box::new(ColorFormatter::new(stdout, &settings)) - } else { - Box::new(PlainTextFormatter::new(stdout)) - }; - let formatter = Mutex::new(formatter); + let color = is_atty; + let formatter = Mutex::new(new_formatter(&settings, color, stdout)); Ui { cwd, + color, formatter, settings, } @@ -65,20 +75,27 @@ impl<'stdout> Ui<'stdout> { &self.settings } - pub fn formatter(&self) -> MutexGuard> { + pub fn new_formatter<'output>( + &self, + output: Box, + ) -> Box { + new_formatter(&self.settings, self.color, output) + } + + pub fn stdout_formatter(&self) -> MutexGuard> { self.formatter.lock().unwrap() } pub fn write(&mut self, text: &str) -> io::Result<()> { - self.formatter().write_str(text) + self.stdout_formatter().write_str(text) } pub fn write_fmt(&mut self, fmt: fmt::Arguments<'_>) -> io::Result<()> { - self.formatter().write_fmt(fmt) + self.stdout_formatter().write_fmt(fmt) } pub fn write_error(&mut self, text: &str) -> io::Result<()> { - let mut formatter = self.formatter(); + let mut formatter = self.stdout_formatter(); formatter.add_label(String::from("error"))?; formatter.write_str(text)?; formatter.remove_label()?; @@ -96,7 +113,7 @@ impl<'stdout> Ui<'stdout> { ) }); let template = crate::template_parser::parse_commit_template(repo, &template_string); - let mut formatter = self.formatter(); + let mut formatter = self.stdout_formatter(); let mut template_writer = TemplateFormatter::new(template, formatter.as_mut()); template_writer.format(commit)?; Ok(())