Support FORCE_COLOR and NO_COLOR

[RedBeard0531]

There are plenty of cases where a process's output isn't directly connected to a TTY, but will still be eventually sent to a TTY. For example, running a test in either ninja or bazel will buffer the output so that things can run in parallel, but still have clean contiguous output for each one rather than a jumbled mess of interleaved output from different processes. This is why tools support flags like --color=always or --force_color. There is also the semi-standard FORCE_COLOR environment variable (and it's mate NO_COLOR.)

I think the right thing for a library to do is for the automatic detection logic to honor FORCE_COLOR/NO_COLOR, but also provide a way for the application to indicate that it really does or does not want color, overriding the library's detection mechanism entirely. This is important both to support application-level flags like --force-color, as well as cases where the application streams into an ostringstream buffer that it intends to later write to stdout. For example, it may want to buffer up a large block of related output then write that to the real stdout under a lock, for the same reason as ninja and bazel, but to prevent interleaving of threads rather than processes.

[jeremy-rifkin]

Hi,
I'm not familiar with FORCE_COLOR and NO_COLOR but I'm open to supporting these as part of the color detection system especially so if other tools support them.
As far as controlling the color output, the library currently has the following facilities:

• stacktrace::print, stacktrace::print_with_snippets, and stacktrace::to_string all have color parameters
• formatter::color_mode has settings to always use color and not use color, in addition to automatic detection
• formatter::format and formatter::print overloads have color parameters

One thing I'm unsure about is how FORCE_COLOR should interact with something like to_string. I think it would be pretty surprising for an environment variable to affect something like that.

[RedBeard0531]

Oh, I don't think to_string() should be affected by FORCE_COLOR. My thought was that it would only affect print() (and similar) when the color mode is automatic, and either the output isn't specified, or possibly if it is explicitly std::cout or std::cerr.

Unfortunately I misread this code which lead to filing a confusing ticket:

cpptrace/src/formatting.cpp

Lines 204 to 214 in 9133b90

	bool should_do_color(std::ostream& stream, detail::optional<bool> color_override) const {
	bool do_color = options.color == color_mode::always || color_override.value_or(false);
	if(
	(options.color == color_mode::automatic || options.color == color_mode::always) &&
	(!color_override || color_override.unwrap() != false) &&
	stream_is_tty(stream)
	) {
	do_color = true;
	}
	return do_color;
	}

My initial read of it was that it would check stream_is_tty() even when color_mode::always, meaning there was no way to force colors when not directly writing to tty. And I suppose it does check for a TTY, but the result of the call doesn't actually change anything because it has already set do_color to true. So there is probably a minor perf bug there that it shouldn't be checking the stream if color_mode is always. In fact that if should probably just be !do_color && colors == automatic && !color_override

Given that there already is a way to force colors even when the output isn't a stream (2 even!) please ignore the parts of the ticket around providing that. FORCE_COLORS/NO_COLORS would still be nice so that applications using the simple automatic detection do the right thing by their users.

[jeremy-rifkin]

So there is probably a minor perf bug there that it shouldn't be checking the stream if color_mode is always

Yes, I'd agree it's not necessary to do the stream check sometimes here. Ultimately this isn't a very performance-critical code path though.

I'll do more research into FORCE_COLORS/NO_COLORS!

[jeremy-rifkin]

I think it'd be good for cpptrace to support these, however, I don't think this is practically implementable in a C/C++ library. I don't know of a way to get environment variables in a thread-safe manner without heroic effort so this will have to go on the backlog for now.

[codeinred]

It might be reasonable to have a cpptrace::use_color variable that’s initialized to 0 by default. If it’s 1, that will force color, and if it’s -1, that will disable color entirely.

namespace cpptrace {
    inline int use_color = 0;
}

And then a function cpptrace::check_no_color(), which will read NO_COLOR and save it to cpptrace::use_color.

This function would not be thread safe, of course, but the intent is that users call check_no_color at the beginning of the program.

Or, alternatively, you check the environment variable on startup:

namespace cpptrace {
    inline int use_color = 
#ifdef CPPTRACE_CHECK_NO_COLOR
        check_no_color();
#else
        0;
#endif
}