ANSI broken on Windows 10

[andrewbanchich]

ansi_term requires Windows 10 programs to explicitly enable ANSI support.

Without doing this, using cargo run or cargo run --release display tracing logs correctly, but when running the binary installed with cargo install --path . it results in broken ANSI strings.

Is this something you think tracing should fix out of the box, should just be communicated in the docs?

[hawkw]

@andrewbanchich hmm. We're currently using the ansi_term crate for ANSI colors. My initial thought is that it should handle this behavior — maybe the best solution is a patch upstream?

[andrewbanchich]

@hawkw It does handle it (see the link I posted) but just not by default.

[hawkw]

Ah, sorry, I misinterpreted your comment (and didn't click the link), my bad. I think we probably want to do that when constructing the subscriber if the target OS is windows, then?

[andrewbanchich]

That makes sense to me. I'll give this a shot and have a pull request soon.

[andrewbanchich]

@hawkw Any thoughts on me swapping out ansi_term for colored? colored has full Windows support and doesn't look like you need to call any special functions conditionally. Plus, it looks nicer overall.

[hawkw]

I'm fine with switching to colored. Currently, we do use ansi-term's Style type to isolate the feature-flagging of ANSI support:

tracing/tracing-subscriber/src/fmt/format/mod.rs

Lines 550 to 559 in 8711f63

	fn bold(&self) -> Style {
	#[cfg(feature = "ansi")]
	{
	if self.ansi {
	return Style::new().bold();
	}
	}
	Style::new()
	}

with a no-op Style type when ansi-term is disabled:

tracing/tracing-subscriber/src/fmt/format/mod.rs

Lines 592 to 603 in 8711f63

	#[cfg(not(feature = "ansi"))]
	struct Style;
	#[cfg(not(feature = "ansi"))]
	impl Style {
	fn new() -> Self {
	Style
	}
	fn paint(&self, d: impl fmt::Display) -> impl fmt::Display {
	d
	}
	}

We can probably do a similar thing with colored's Colorize trait?

[hawkw]

hmm, upon taking a closer look, it looks like colored's methods always return a ColoredString struct which appears to always allocate a String, even when the input is borrowed. This also means that formatting Debug/Display happens eagerly, writing into the string, rather than lazily (allowing us to write directly into the per-thread buffer that tracing-subscriber::fmt creates for each thread that logs).

A big part of how we've optimized the performance of logging events is by avoiding additional string allocations, and writing directly to the buffer, which is reused without deallocating — in many cases, once the buffer is "warmed up" (i.e. it has previously allocated enough length to hold the line we are currently writing), logging an event should be allocation-free. It seems like switching to colored breaks this property, which might have a noticeable performance impact...

[andrewhickman]

termcolor has a buffer type which supports writing colored text to a buffer and clearing afterwards to preserve the allocations. Maybe that would be suitable here?

[Chaoses-Ib]

termcolor also supports Windows 7 and 8 according to rust-lang/rust#55769, while ansi-term only works on Windows 10+.

Edit: anstream also has legacy Windows support, and is possibly a better choice than termcolor: rust-lang/cargo#12627