Skip to content

Commit

Permalink
Reset the terminal color before the newline for diagnostics
Browse files Browse the repository at this point in the history
When printing colored diagnostics, we need to reset the terminal before
emitting the newline, not after. Otherwise it gets line-buffered and the
color won't reset until the next line is printed or the compiler exits.

Normally this isn't a problem, but when running rustc in parallel with
other processes (e.g. `make -j4`) this can cause the color to leak
to other lines.
  • Loading branch information
lilyball committed May 20, 2014
1 parent e8c579e commit b991bbe
Showing 1 changed file with 21 additions and 2 deletions.
23 changes: 21 additions & 2 deletions src/libsyntax/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,27 @@ fn print_maybe_styled(w: &mut EmitterWriter,
match w.dst {
Terminal(ref mut t) => {
try!(t.attr(color));
try!(t.write_str(msg));
try!(t.reset());
// If `msg` ends in a newline, we need to reset the color before
// the newline. We're making the assumption that we end up writing
// to a `LineBufferedWriter`, which means that emitting the reset
// after the newline ends up buffering the reset until we print
// another line or exit. Buffering the reset is a problem if we're
// sharing the terminal with any other programs (e.g. other rustc
// instances via `make -jN`).
//
// Note that if `msg` contains any internal newlines, this will
// result in the `LineBufferedWriter` flushing twice instead of
// once, which still leaves the opportunity for interleaved output
// to be miscolored. We assume this is rare enough that we don't
// have to worry about it.
if msg.ends_with("\n") {
try!(t.write_str(msg.slice_to(msg.len()-1)));
try!(t.reset());
try!(t.write_str("\n"));
} else {
try!(t.write_str(msg));
try!(t.reset());
}
Ok(())
}
Raw(ref mut w) => {
Expand Down

5 comments on commit b991bbe

@bors
Copy link
Contributor

@bors bors commented on b991bbe May 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from alexcrichton
at lilyball@b991bbe

@bors
Copy link
Contributor

@bors bors commented on b991bbe May 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging kballard/rust/diagnostic_color_newline = b991bbe into auto

@bors
Copy link
Contributor

@bors bors commented on b991bbe May 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kballard/rust/diagnostic_color_newline = b991bbe merged ok, testing candidate = 6291955

@bors
Copy link
Contributor

@bors bors commented on b991bbe May 20, 2014

@bors
Copy link
Contributor

@bors bors commented on b991bbe May 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 6291955

Please sign in to comment.