Skip to content

Commit ba344d3

Browse files
Nero5023facebook-github-bot
authored andcommitted
Handle stdout is not compatible with tty case
Summary: RFC: https://fb.workplace.com/groups/buck2dev/permalink/3976662752621767/ This diff handles issue when aux_stream (stdout) is not compatible with tty. It just output the raw text without escape sequence to clear the line, etc Reviewed By: cjhopman Differential Revision: D73564704 fbshipit-source-id: fb6dc6f5aedb279d49eb1ff74c8cd514e7db3e09
1 parent 2e0e1ba commit ba344d3

File tree

5 files changed

+120
-21
lines changed

5 files changed

+120
-21
lines changed

src/builder.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use std::io::Write;
1313
use crate::Dimensions;
1414
use crate::SuperConsole;
1515
use crate::output::BlockingSuperConsoleOutput;
16+
use crate::output::IsTtyWrite;
1617
use crate::output::NonBlockingSuperConsoleOutput;
1718
use crate::output::SuperConsoleOutput;
1819

@@ -22,7 +23,7 @@ pub struct Builder {
2223
// The stream that superconsole writes to by default (emit output + canvas). By default is stderr.
2324
stream: Box<dyn Write + Send + 'static + Sync>,
2425
// The stream that superconsole writes to for auxiliary output. By default is stdout.
25-
aux_stream: Box<dyn Write + Send + 'static + Sync>,
26+
aux_stream: Box<dyn IsTtyWrite + Send + 'static + Sync>,
2627
}
2728

2829
impl Default for Builder {

src/content/lines.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use std::cmp::Ordering;
1212
use std::fmt;
1313
use std::fmt::Display;
1414
use std::fmt::Formatter;
15+
use std::io::Write;
1516
use std::iter;
1617
use std::mem;
1718

@@ -318,6 +319,16 @@ impl Lines {
318319
Ok(())
319320
}
320321

322+
/// Render the lines without an escape sequence to clear the line.
323+
/// It will clear the lines at the end.
324+
pub(crate) fn render_raw(&mut self, writer: &mut Vec<u8>) -> anyhow::Result<()> {
325+
for line in self.0.iter() {
326+
writeln!(writer, "{}", line.render())?;
327+
}
328+
self.0.clear();
329+
Ok(())
330+
}
331+
321332
pub(crate) fn lines_equal(&self, other: &Self) -> usize {
322333
self.0
323334
.iter()

src/output.rs

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use crossbeam_channel::Receiver;
1717
use crossbeam_channel::Sender;
1818
use crossbeam_channel::bounded;
1919
use crossbeam_channel::unbounded;
20+
use crossterm::tty::IsTty;
2021

2122
use crate::Dimensions;
2223

@@ -29,6 +30,10 @@ pub enum OutputTarget {
2930
Aux,
3031
}
3132

33+
pub trait IsTtyWrite: IsTty + Write {}
34+
35+
impl<T: IsTty + Write> IsTtyWrite for T {}
36+
3237
pub trait SuperConsoleOutput: Send + Sync + 'static {
3338
/// Called before rendering will occur. This has a chance to prevent rendering by returning
3439
/// false.
@@ -46,6 +51,11 @@ pub trait SuperConsoleOutput: Send + Sync + 'static {
4651
self.output(buffer)
4752
}
4853

54+
/// Check if auxillary stream is tty
55+
fn aux_stream_is_tty(&self) -> bool {
56+
true
57+
}
58+
4959
/// How big is the terminal to write to.
5060
fn terminal_size(&self) -> anyhow::Result<Dimensions> {
5161
Ok(crossterm::terminal::size()?.into())
@@ -66,13 +76,13 @@ pub struct BlockingSuperConsoleOutput {
6676
/// Stream to write to.
6777
stream: Box<dyn Write + Send + 'static + Sync>,
6878
/// Auxiliary stream to write to.
69-
aux_stream: Box<dyn Write + Send + 'static + Sync>,
79+
aux_stream: Box<dyn IsTtyWrite + Send + 'static + Sync>,
7080
}
7181

7282
impl BlockingSuperConsoleOutput {
7383
pub fn new(
7484
stream: Box<dyn Write + Send + 'static + Sync>,
75-
aux_stream: Box<dyn Write + Send + 'static + Sync>,
85+
aux_stream: Box<dyn IsTtyWrite + Send + 'static + Sync>,
7686
) -> Self {
7787
Self { stream, aux_stream }
7888
}
@@ -87,6 +97,10 @@ impl SuperConsoleOutput for BlockingSuperConsoleOutput {
8797
self.output_to(buffer, OutputTarget::Main)
8898
}
8999

100+
fn aux_stream_is_tty(&self) -> bool {
101+
self.aux_stream.is_tty()
102+
}
103+
90104
fn output_to(&mut self, buffer: Vec<u8>, target: OutputTarget) -> anyhow::Result<()> {
91105
match target {
92106
OutputTarget::Main => {
@@ -127,30 +141,33 @@ pub(crate) struct NonBlockingSuperConsoleOutput {
127141
/// The thread doing the writing. It owns the other end of the aforementioned channels and will
128142
/// exit when the data sender is closed.
129143
handle: JoinHandle<()>,
144+
/// The auxillary output is compatible with tty
145+
aux_compatible: bool,
130146
}
131147

132148
impl NonBlockingSuperConsoleOutput {
133149
pub fn new(
134150
stream: Box<dyn Write + Send + 'static + Sync>,
135-
aux_stream: Box<dyn Write + Send + 'static + Sync>,
151+
aux_stream: Box<dyn IsTtyWrite + Send + 'static + Sync>,
136152
) -> anyhow::Result<Self> {
137153
Self::new_for_writer(stream, aux_stream)
138154
}
139155

140156
fn new_for_writer(
141157
mut stream: Box<dyn Write + Send + 'static + Sync>,
142-
mut aux_stream: Box<dyn Write + Send + 'static + Sync>,
158+
mut aux_stream: Box<dyn IsTtyWrite + Send + 'static + Sync>,
143159
) -> anyhow::Result<Self> {
144160
let (sender, receiver) = bounded::<(Vec<u8>, OutputTarget)>(1);
145161
let (error_sender, errors) = unbounded::<io::Error>();
162+
let aux_compatible = aux_stream.is_tty();
146163

147164
let handle = std::thread::Builder::new()
148165
.name("superconsole-io".to_owned())
149166
.spawn(move || {
150167
for (data, output_target) in receiver.into_iter() {
151168
let out_stream = match output_target {
152169
OutputTarget::Main => &mut stream,
153-
OutputTarget::Aux => &mut aux_stream,
170+
OutputTarget::Aux => &mut aux_stream as &mut dyn Write,
154171
};
155172
match out_stream.write_all(&data).and_then(|()| stream.flush()) {
156173
Ok(()) => {}
@@ -168,6 +185,7 @@ impl NonBlockingSuperConsoleOutput {
168185
sender,
169186
errors,
170187
handle,
188+
aux_compatible,
171189
})
172190
}
173191
}
@@ -198,12 +216,17 @@ impl SuperConsoleOutput for NonBlockingSuperConsoleOutput {
198216
Ok(())
199217
}
200218

219+
fn aux_stream_is_tty(&self) -> bool {
220+
self.aux_compatible
221+
}
222+
201223
/// Notify our writer thread that no further writes are expected. Wait for it to flush.
202224
fn finalize(self: Box<Self>) -> anyhow::Result<()> {
203225
let Self {
204226
sender,
205227
errors,
206228
handle,
229+
aux_compatible: _,
207230
} = *self;
208231
drop(sender);
209232

@@ -247,6 +270,12 @@ mod tests {
247270
}
248271
}
249272

273+
impl IsTty for TestWriter {
274+
fn is_tty(&self) -> bool {
275+
true
276+
}
277+
}
278+
250279
impl Write for TestWriter {
251280
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
252281
self.sender

src/superconsole.rs

Lines changed: 58 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -241,17 +241,24 @@ impl SuperConsole {
241241
// We don't trust the child to not truncate the result.
242242
canvas.shrink_lines_to_dimensions(size);
243243

244+
// The closure to compute the limit to render the emit lines above the canvas.
244245
// Render at most a single frame if this not the last render.
245246
// Does not buffer if there is a ridiculous amount of data.
246-
let mut limit = match mode {
247-
DrawMode::Normal if !is_big(&self.to_emit, &self.aux_to_emit) => {
248-
let limit = size.height.saturating_sub(canvas.len());
249-
// arbitrary value picked so we don't starve `emit` on small terminal sizes.
250-
Some(cmp::max(limit, MINIMUM_EMIT))
247+
let compute_limit = |buf0: &Lines, buf1: &Lines| {
248+
match mode {
249+
DrawMode::Normal if !is_big(buf0, buf1) => {
250+
let limit = size.height.saturating_sub(canvas.len());
251+
// arbitrary value picked so we don't starve `emit` on small terminal sizes.
252+
Some(cmp::max(limit, MINIMUM_EMIT))
253+
}
254+
_ => None,
251255
}
252-
_ => None,
253256
};
254257

258+
// Render at most a single frame if this not the last render.
259+
// Does not buffer if there is a ridiculous amount of data.
260+
let mut limit = compute_limit(&self.to_emit, &self.aux_to_emit);
261+
255262
// How much of the canvas hasn't changed, so I can avoid overwriting
256263
// and thus avoid flickering things like URL's in VS Code terminal.
257264
let reuse_prefix = if self.to_emit.is_empty() && self.aux_to_emit.is_empty() {
@@ -265,15 +272,25 @@ impl SuperConsole {
265272
Self::clear_canvas_pre(&mut buffer, self.canvas_contents.len() - reuse_prefix)?;
266273

267274
if !self.aux_to_emit.is_empty() {
268-
// If we have aux_to_emit, we need to output the main output (stderr by default) first
269-
// and flushed, so that we can make sure the all output order is correct.
270-
self.output.output(buffer)?;
271-
let mut aux_buffer = Vec::new();
272-
limit = self.aux_to_emit.render_with_limit(&mut aux_buffer, limit)?;
273-
self.output.output_to(aux_buffer, OutputTarget::Aux)?;
274-
275-
// Since output is moved at `self.output.output(buffer)`, we need to new a new buffer
276-
buffer = Vec::new();
275+
if self.output.aux_stream_is_tty() {
276+
// If we have aux_to_emit and the aux stream is tty, we need to output the main output (stderr by default) first
277+
// and flushed, so that we can make sure the all output order is correct.
278+
self.output.output(buffer)?;
279+
let mut aux_buffer = Vec::new();
280+
limit = self.aux_to_emit.render_with_limit(&mut aux_buffer, limit)?;
281+
self.output.output_to(aux_buffer, OutputTarget::Aux)?;
282+
283+
// Since output is moved at `self.output.output(buffer)`, we need to new a new buffer
284+
buffer = Vec::new();
285+
} else {
286+
// If the aux stream is not tty, we don't need to render the line, we just output to the auxillary output
287+
let mut output_buffer = Vec::new();
288+
self.aux_to_emit.render_raw(&mut output_buffer)?;
289+
self.output.output_to(output_buffer, OutputTarget::Aux)?;
290+
291+
// Since we clear the aux_to_emit, we need to recompute the `limit`
292+
limit = compute_limit(&self.to_emit, &self.aux_to_emit);
293+
}
277294
}
278295

279296
self.to_emit.render_with_limit(&mut buffer, limit)?;
@@ -299,6 +316,7 @@ mod tests {
299316
use crate::testing::TestOutput;
300317
use crate::testing::frame_contains;
301318
use crate::testing::test_console;
319+
use crate::testing::test_console_aux_incompatible;
302320

303321
#[derive(AsRef, Debug)]
304322
#[allow(dead_code)]
@@ -587,4 +605,29 @@ mod tests {
587605

588606
Ok(())
589607
}
608+
609+
#[test]
610+
fn test_emit_aux_with_aux_incompatible_output() -> anyhow::Result<()> {
611+
let mut console = test_console_aux_incompatible();
612+
613+
let root = Echo(Lines(vec![vec!["state"].try_into()?]));
614+
console.emit_aux(Lines(vec![vec!["aux line 1"].try_into()?]));
615+
console.emit_aux(Lines(vec![vec!["aux line 2"].try_into()?]));
616+
console.render(&root)?;
617+
618+
// Since we emit aux, we don't output the whole frame once in TestConsole
619+
let frame: Vec<u8> = console
620+
.test_output_mut()?
621+
.frames
622+
.iter()
623+
.flatten()
624+
.copied()
625+
.collect();
626+
println!("frame: {:?}", String::from_utf8_lossy(&frame));
627+
assert!(frame_contains(&frame, "state"));
628+
// Since aux output is incompatible with tty, we don't output the any escape sequence to clear the line.
629+
assert!(frame_contains(&frame, "aux line 1\naux line 2"));
630+
631+
Ok(())
632+
}
590633
}

src/testing.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ pub struct TestOutput {
2525
pub terminal_size: Dimensions,
2626
/// The frames that were written to this output.
2727
pub frames: Vec<Vec<u8>>,
28+
/// Whether auxiliary output is compatible with tty
29+
aux_stream_is_tty: bool,
2830
}
2931

3032
impl TestOutput {
@@ -63,6 +65,10 @@ impl SuperConsoleOutput for TestOutput {
6365
}
6466
}
6567

68+
fn aux_stream_is_tty(&self) -> bool {
69+
self.aux_stream_is_tty
70+
}
71+
6672
fn terminal_size(&self) -> anyhow::Result<Dimensions> {
6773
Ok(self.terminal_size)
6874
}
@@ -102,6 +108,14 @@ impl SuperConsoleTestingExt for SuperConsole {
102108
}
103109

104110
pub fn test_console() -> SuperConsole {
111+
test_console_inner(true)
112+
}
113+
114+
pub fn test_console_aux_incompatible() -> SuperConsole {
115+
test_console_inner(false)
116+
}
117+
118+
fn test_console_inner(aux_stream_is_tty: bool) -> SuperConsole {
105119
let size = Dimensions {
106120
width: 80,
107121
height: 80,
@@ -112,6 +126,7 @@ pub fn test_console() -> SuperConsole {
112126
should_render: true,
113127
terminal_size: size,
114128
frames: Vec::new(),
129+
aux_stream_is_tty,
115130
}),
116131
)
117132
}

0 commit comments

Comments
 (0)