ok/jj
1
0
Fork 0
forked from mirrors/jj

formatter: reset colors on early drop

If we create a `ColorFormatter`, add some labels to it, print
something using the configured style, and then return early because of
an error, we would leave the terminal in a bad state. I think many
shells reset color codes after a command returns, but let's still do
our best.
This commit is contained in:
Martin von Zweigbergk 2023-11-01 17:53:13 -07:00 committed by Martin von Zweigbergk
parent 2f6cce1e4f
commit 46a8afe144
3 changed files with 45 additions and 2 deletions

View file

@ -216,7 +216,7 @@ impl Style {
} }
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
pub struct ColorFormatter<W> { pub struct ColorFormatter<W: Write> {
output: W, output: W,
rules: Arc<Rules>, rules: Arc<Rules>,
/// The stack of currently applied labels. These determine the desired /// The stack of currently applied labels. These determine the desired
@ -460,6 +460,15 @@ impl<W: Write> Formatter for ColorFormatter<W> {
} }
} }
impl<W: Write> Drop for ColorFormatter<W> {
fn drop(&mut self) {
// If a `ColorFormatter` was dropped without popping all labels first (perhaps
// because of an error), let's still try to reset any currently active style.
self.labels.clear();
self.write_new_style().ok();
}
}
/// Like buffered formatter, but records `push`/`pop_label()` calls. /// Like buffered formatter, but records `push`/`pop_label()` calls.
/// ///
/// This allows you to manipulate the recorded data without losing labels. /// This allows you to manipulate the recorded data without losing labels.
@ -646,6 +655,7 @@ mod tests {
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
formatter.write_str("\n").unwrap(); formatter.write_str("\n").unwrap();
} }
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @r###" insta::assert_snapshot!(String::from_utf8(output).unwrap(), @r###"
 black   black 
 red   red 
@ -682,6 +692,7 @@ mod tests {
formatter.write_str(" inside ").unwrap(); formatter.write_str(" inside ").unwrap();
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
formatter.write_str(" after ").unwrap(); formatter.write_str(" after ").unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" before  inside  after "); insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" before  inside  after ");
} }
@ -726,6 +737,7 @@ mod tests {
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
formatter.write_str("\n").unwrap(); formatter.write_str("\n").unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @r###" insta::assert_snapshot!(String::from_utf8(output).unwrap(), @r###"
 fg only   fg only 
 bg only   bg only 
@ -754,6 +766,7 @@ mod tests {
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
formatter.write_str(" not bold again ").unwrap(); formatter.write_str(" not bold again ").unwrap();
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" not bold  bold  not bold again "); insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" not bold  bold  not bold again ");
} }
@ -776,6 +789,7 @@ mod tests {
formatter.write_str("second").unwrap(); formatter.write_str("second").unwrap();
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
formatter.write_str("after").unwrap(); formatter.write_str("after").unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @"beforefirstsecondafter"); insta::assert_snapshot!(String::from_utf8(output).unwrap(), @"beforefirstsecondafter");
} }
@ -794,6 +808,7 @@ mod tests {
.write_str("\x1b[1mnot actually bold\x1b[0m") .write_str("\x1b[1mnot actually bold\x1b[0m")
.unwrap(); .unwrap();
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @"␛[1mnot actually bold␛[0m"); insta::assert_snapshot!(String::from_utf8(output).unwrap(), @"␛[1mnot actually bold␛[0m");
} }
@ -820,6 +835,7 @@ mod tests {
formatter.write_str(" after inner ").unwrap(); formatter.write_str(" after inner ").unwrap();
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
formatter.write_str(" after outer ").unwrap(); formatter.write_str(" after outer ").unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), insta::assert_snapshot!(String::from_utf8(output).unwrap(),
@" before outer  before inner  inside inner  after inner  after outer "); @" before outer  before inner  inside inner  after inner  after outer ");
} }
@ -841,6 +857,7 @@ mod tests {
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
formatter.write_str(" not colored ").unwrap(); formatter.write_str(" not colored ").unwrap();
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), insta::assert_snapshot!(String::from_utf8(output).unwrap(),
@" not colored  colored  not colored "); @" not colored  colored  not colored ");
} }
@ -885,10 +902,11 @@ mod tests {
formatter.write_str(" default bg, ").unwrap(); formatter.write_str(" default bg, ").unwrap();
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
formatter.write_str(" and back.").unwrap(); formatter.write_str(" and back.").unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), insta::assert_snapshot!(String::from_utf8(output).unwrap(),
@r###" @r###"
Blue on yellow,  default fg,  and back. Blue on yellow,  default fg,  and back.
Blue on yellow,  default bg,  and back. Blue on yellow,  default bg,  and back.
"###); "###);
} }
@ -908,6 +926,7 @@ mod tests {
formatter.write_str(" hello ").unwrap(); formatter.write_str(" hello ").unwrap();
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), insta::assert_snapshot!(String::from_utf8(output).unwrap(),
@" hello "); @" hello ");
} }
@ -927,6 +946,7 @@ mod tests {
formatter.write_str(" hello ").unwrap(); formatter.write_str(" hello ").unwrap();
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" hello "); insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" hello ");
} }
@ -954,10 +974,29 @@ mod tests {
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
formatter.write_str(" a2 ").unwrap(); formatter.write_str(" a2 ").unwrap();
formatter.pop_label().unwrap(); formatter.pop_label().unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), insta::assert_snapshot!(String::from_utf8(output).unwrap(),
@" a1  b1  c  b2  a2 "); @" a1  b1  c  b2  a2 ");
} }
#[test]
fn test_color_formatter_dropped() {
// Test that the style gets reset if the formatter is dropped without popping
// all labels.
let config = config_from_string(
r#"
colors.outer = "green"
"#,
);
let mut output: Vec<u8> = vec![];
let mut formatter = ColorFormatter::for_config(&mut output, &config).unwrap();
formatter.push_label("outer").unwrap();
formatter.push_label("inner").unwrap();
formatter.write_str(" inside ").unwrap();
drop(formatter);
insta::assert_snapshot!(String::from_utf8(output).unwrap(), @" inside ");
}
#[test] #[test]
fn test_format_recorder() { fn test_format_recorder() {
let mut recorder = FormatRecorder::new(); let mut recorder = FormatRecorder::new();
@ -977,6 +1016,7 @@ mod tests {
let mut output: Vec<u8> = vec![]; let mut output: Vec<u8> = vec![];
let mut formatter = ColorFormatter::for_config(&mut output, &config).unwrap(); let mut formatter = ColorFormatter::for_config(&mut output, &config).unwrap();
recorder.replay(&mut formatter).unwrap(); recorder.replay(&mut formatter).unwrap();
drop(formatter);
insta::assert_snapshot!( insta::assert_snapshot!(
String::from_utf8(output).unwrap(), String::from_utf8(output).unwrap(),
@" outer1  inner1 inner2  outer2 "); @" outer1  inner1 inner2  outer2 ");
@ -990,6 +1030,7 @@ mod tests {
write!(formatter, "<<{}>>", str::from_utf8(data).unwrap()) write!(formatter, "<<{}>>", str::from_utf8(data).unwrap())
}) })
.unwrap(); .unwrap();
drop(formatter);
insta::assert_snapshot!( insta::assert_snapshot!(
String::from_utf8(output).unwrap(), String::from_utf8(output).unwrap(),
@"<< outer1 >><< inner1 inner2 >><< outer2 >>"); @"<< outer1 >><< inner1 inner2 >><< outer2 >>");

View file

@ -946,6 +946,7 @@ mod tests {
let mut output = Vec::new(); let mut output = Vec::new();
let mut formatter = ColorFormatter::new(&mut output, self.color_rules.clone().into()); let mut formatter = ColorFormatter::new(&mut output, self.color_rules.clone().into());
template.format(&(), &mut formatter).unwrap(); template.format(&(), &mut formatter).unwrap();
drop(formatter);
String::from_utf8(output).unwrap() String::from_utf8(output).unwrap()
} }
} }

View file

@ -275,6 +275,7 @@ mod tests {
let mut output = Vec::new(); let mut output = Vec::new();
let mut formatter = ColorFormatter::for_config(&mut output, &config).unwrap(); let mut formatter = ColorFormatter::for_config(&mut output, &config).unwrap();
write(&mut formatter).unwrap(); write(&mut formatter).unwrap();
drop(formatter);
String::from_utf8(output).unwrap() String::from_utf8(output).unwrap()
} }