From 8bb806e7b6dcfa9474c6be940e397cbcd9f41faf Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sun, 13 Oct 2024 20:45:25 +0900 Subject: [PATCH] formatter: flush color escape sequence when raw() stream is requested This ensures that the data printed through the raw stream is colorized if the formatter already had color labels, and if the raw data doesn't reset the surrounding color. This would only matter in templates containing label(.., raw_escape_sequence() ..) expression. Fixes #4631 --- cli/src/commands/evolog.rs | 2 +- cli/src/commands/log.rs | 2 +- cli/src/commands/operation/diff.rs | 2 +- cli/src/commands/operation/log.rs | 2 +- cli/src/diff_util.rs | 17 ++++++-------- cli/src/formatter.rs | 36 +++++++++++++++--------------- cli/src/templater.rs | 5 ++--- 7 files changed, 31 insertions(+), 35 deletions(-) diff --git a/cli/src/commands/evolog.rs b/cli/src/commands/evolog.rs index 28fbac790..0f40fa161 100644 --- a/cli/src/commands/evolog.rs +++ b/cli/src/commands/evolog.rs @@ -142,7 +142,7 @@ pub(crate) fn cmd_evolog( commits.truncate(n); } if !args.no_graph { - let mut raw_output = formatter.raw(); + let mut raw_output = formatter.raw()?; let mut graph = get_graphlog(graph_style, raw_output.as_mut()); for commit in commits { let edges = commit diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs index afbb74c0a..8c783ef12 100644 --- a/cli/src/commands/log.rs +++ b/cli/src/commands/log.rs @@ -169,7 +169,7 @@ pub(crate) fn cmd_log( let limit = args.limit.or(args.deprecated_limit).unwrap_or(usize::MAX); if !args.no_graph { - let mut raw_output = formatter.raw(); + let mut raw_output = formatter.raw()?; let mut graph = get_graphlog(graph_style, raw_output.as_mut()); let forward_iter = TopoGroupedGraphIterator::new(revset.iter_graph()); let iter: Box> = if args.reversed { diff --git a/cli/src/commands/operation/diff.rs b/cli/src/commands/operation/diff.rs index 8a86ccbb9..e3b373c80 100644 --- a/cli/src/commands/operation/diff.rs +++ b/cli/src/commands/operation/diff.rs @@ -197,7 +197,7 @@ pub fn show_op_diff( writeln!(formatter, "Changed commits:") })?; if let Some(graph_style) = graph_style { - let mut raw_output = formatter.raw(); + let mut raw_output = formatter.raw()?; let mut graph = get_graphlog(graph_style, raw_output.as_mut()); let graph_iter = diff --git a/cli/src/commands/operation/log.rs b/cli/src/commands/operation/log.rs index cc36d4cf7..03308172e 100644 --- a/cli/src/commands/operation/log.rs +++ b/cli/src/commands/operation/log.rs @@ -197,7 +197,7 @@ fn do_op_log( let limit = args.limit.or(args.deprecated_limit).unwrap_or(usize::MAX); let iter = op_walk::walk_ancestors(slice::from_ref(current_op)).take(limit); if !args.no_graph { - let mut raw_output = formatter.raw(); + let mut raw_output = formatter.raw()?; let mut graph = get_graphlog(graph_style, raw_output.as_mut()); for op in iter { let op = op?; diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index 7c6b4b46f..495e06ae4 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -366,15 +366,11 @@ impl<'a> DiffRenderer<'a> { tool, ) } - DiffToolMode::Dir => generate_diff( - ui, - formatter.raw().as_mut(), - from_tree, - to_tree, - matcher, - tool, - ) - .map_err(DiffRenderError::DiffGenerate), + DiffToolMode::Dir => { + let mut writer = formatter.raw()?; + generate_diff(ui, writer.as_mut(), from_tree, to_tree, matcher, tool) + .map_err(DiffRenderError::DiffGenerate) + } }?; } } @@ -1102,9 +1098,10 @@ pub fn show_file_by_file_diff( let left_path = create_file(left_path, &left_wc_dir, left_value)?; let right_path = create_file(right_path, &right_wc_dir, right_value)?; + let mut writer = formatter.raw()?; invoke_external_diff( ui, - formatter.raw().as_mut(), + writer.as_mut(), tool, &maplit::hashmap! { "left" => left_path.to_str().expect("temp_dir should be valid utf-8"), diff --git a/cli/src/formatter.rs b/cli/src/formatter.rs index 0a70554fe..400597395 100644 --- a/cli/src/formatter.rs +++ b/cli/src/formatter.rs @@ -34,7 +34,7 @@ use itertools::Itertools; pub trait Formatter: Write { /// Returns the backing `Write`. This is useful for writing data that is /// already formatted, such as in the graphical log. - fn raw(&mut self) -> Box; + fn raw(&mut self) -> io::Result>; fn push_label(&mut self, label: &str) -> io::Result<()>; @@ -203,8 +203,8 @@ impl Write for PlainTextFormatter { } impl Formatter for PlainTextFormatter { - fn raw(&mut self) -> Box { - Box::new(self.output.by_ref()) + fn raw(&mut self) -> io::Result> { + Ok(Box::new(self.output.by_ref())) } fn push_label(&mut self, _label: &str) -> io::Result<()> { @@ -238,8 +238,8 @@ impl Write for SanitizingFormatter { } impl Formatter for SanitizingFormatter { - fn raw(&mut self) -> Box { - Box::new(self.output.by_ref()) + fn raw(&mut self) -> io::Result> { + Ok(Box::new(self.output.by_ref())) } fn push_label(&mut self, _label: &str) -> io::Result<()> { @@ -541,8 +541,9 @@ impl Write for ColorFormatter { } impl Formatter for ColorFormatter { - fn raw(&mut self) -> Box { - Box::new(self.output.by_ref()) + fn raw(&mut self) -> io::Result> { + self.write_new_style()?; + Ok(Box::new(self.output.by_ref())) } fn push_label(&mut self, label: &str) -> io::Result<()> { @@ -626,8 +627,7 @@ impl FormatRecorder { FormatOp::PushLabel(label) => formatter.push_label(label)?, FormatOp::PopLabel => formatter.pop_label()?, FormatOp::RawEscapeSequence(raw_escape_sequence) => { - // TODO(#4631): process "buffered" labels. - formatter.raw().write_all(raw_escape_sequence)?; + formatter.raw()?.write_all(raw_escape_sequence)?; } } } @@ -660,8 +660,8 @@ impl<'a> Write for RawEscapeSequenceRecorder<'a> { } impl Formatter for FormatRecorder { - fn raw(&mut self) -> Box { - Box::new(RawEscapeSequenceRecorder(self)) + fn raw(&mut self) -> io::Result> { + Ok(Box::new(RawEscapeSequenceRecorder(self))) } fn push_label(&mut self, label: &str) -> io::Result<()> { @@ -1267,21 +1267,21 @@ mod tests { fn test_raw_format_recorder() { // Note: similar to test_format_recorder above let mut recorder = FormatRecorder::new(); - write!(recorder.raw(), " outer1 ").unwrap(); + write!(recorder.raw().unwrap(), " outer1 ").unwrap(); recorder.push_label("inner").unwrap(); - write!(recorder.raw(), " inner1 ").unwrap(); - write!(recorder.raw(), " inner2 ").unwrap(); + write!(recorder.raw().unwrap(), " inner1 ").unwrap(); + write!(recorder.raw().unwrap(), " inner2 ").unwrap(); recorder.pop_label().unwrap(); - write!(recorder.raw(), " outer2 ").unwrap(); + write!(recorder.raw().unwrap(), " outer2 ").unwrap(); - // No non-raw output to label + // Replayed raw escape sequences are labeled. let config = config_from_string(r#" colors.inner = "red" "#); let mut output: Vec = vec![]; let mut formatter = ColorFormatter::for_config(&mut output, &config, false).unwrap(); recorder.replay(&mut formatter).unwrap(); drop(formatter); insta::assert_snapshot!( - String::from_utf8(output).unwrap(), @" outer1 inner1 inner2 outer2 "); + String::from_utf8(output).unwrap(), @" outer1  inner1 inner2  outer2 "); let mut output: Vec = vec![]; let mut formatter = ColorFormatter::for_config(&mut output, &config, false).unwrap(); @@ -1295,6 +1295,6 @@ mod tests { .unwrap(); drop(formatter); insta::assert_snapshot!( - String::from_utf8(output).unwrap(), @" outer1 inner1 inner2 outer2 "); + String::from_utf8(output).unwrap(), @" outer1  inner1 inner2  outer2 "); } } diff --git a/cli/src/templater.rs b/cli/src/templater.rs index 718ac2dde..05f7db403 100644 --- a/cli/src/templater.rs +++ b/cli/src/templater.rs @@ -190,8 +190,7 @@ pub struct RawEscapeSequenceTemplate(pub T); impl Template for RawEscapeSequenceTemplate { fn format(&self, formatter: &mut TemplateFormatter) -> io::Result<()> { let rewrap = formatter.rewrap_fn(); - let mut raw_formatter = PlainTextFormatter::new(formatter.raw()); - // TODO(#4631): process "buffered" labels. + let mut raw_formatter = PlainTextFormatter::new(formatter.raw()?); self.0.format(&mut rewrap(&mut raw_formatter)) } } @@ -709,7 +708,7 @@ impl<'a> TemplateFormatter<'a> { move |formatter| TemplateFormatter::new(formatter, error_handler) } - pub fn raw(&mut self) -> Box { + pub fn raw(&mut self) -> io::Result> { self.formatter.raw() }