std.io.tty: cleanup detectConfig

[dweiller]

This PR adds fn std.io.tty.detectConfigForce(File, bool) Config which accepts an extra parameter to force coloring if the platform supports it. There are two code paths where .no_color will be returned: if the platform is wasi, or on windows if the windows.kernel32.GetConsoleScreenBufferInfo() test fails - other options in this case could be to return an error, or .escape_codes (though this might be unexpected). If you want to force escape codes on wasi for some reason, you don't need to do TTY detection, just use .escape_codes directly.

[dweiller]

I'm not sure I understand the CI failure - it looks like it's something to do with docgen. The error says it's a problem with wasi not supporting hasEnvVarConstants, but that doesn't make sense to me as the changes should not have changed the behaviour of std.io.tty.detectConfig() at all.

[dweiller]

Or is it something specifically to do with bootstrapping? It looks like build-debug/zig2 is the executable that's being used during the failure.

[andrewrk]

I pushed a commit with my opinion of how it should be. What do you think?

[dweiller]

This seems like a nicer way to go. I'm unsure what the ZIG_DEBUG_COLOR environment variable was for - is there somewhere else in the codebase that was using it?

[dweiller]

I originally had this behaviour (forcing colour on WASI returning .escape_codes but I think the least surprising behaviour is for detectConfigForce to not output escapes codes on platforms that don't support them. If someone actually wants them in an environment that doesn't support them they should just not call detectConfig*.

[andrewrk]

When color is forced on, it is irrelevant whether a platform supports it or not. The intention of forcing it on is that the output stream will be interpreted to have escape codes in them, and handled appropriately. This can be done on WASI, or Windows, or any platform.

[dweiller]

I wasn't actually sure how windows works with colour so I left the previous behaviour. If windows would support escape codes here this seems fine, if not similar to WASI I'd suggest returning .no_color unconditionally.

[andrewrk]

This seems like a nicer way to go. I'm unsure what the ZIG_DEBUG_COLOR environment variable was for - is there somewhere else in the codebase that was using it?

It was moved from std.debug in #15806.

[dweiller]

Looks like ZIG_DEBUG_COLOR is referenced in 3 others files:

~/S/z/src (master) > rg ZIG_DEBUG_COLOR
lib/build_runner.zig
285:        .escape_codes => try builder.env_map.put("ZIG_DEBUG_COLOR", "1"),

lib/std/io/tty.zig
16:    } else if (process.hasEnvVarConstant("ZIG_DEBUG_COLOR")) {


test/src/StackTrace.zig
84:    run.removeEnvironmentVariable("ZIG_DEBUG_COLOR");

doc/docgen.zig
1308:    try env_map.put("ZIG_DEBUG_COLOR", "1");

[andrewrk]

ZIG_DEBUG_COLOR was renamed to YES_COLOR

[dweiller]

whoops

[andrewrk]

Alright, I have it figured out. Ripgrep's --color <when> option is bad.

"always" should not be an option, because it is ambiguous.

The proper CLI options to accept are:

• always off (no_color)
• ansi (escape_codes)
• auto/default

As for std.io.tty.detectConfig, there should not be a "force" variant of the function. It should be how it was before, with only 1 function but with the logic cleaned up as I have done in the commit here.

Then your CLI application logic will look almost exactly how Zig's CLI logic already looks:

zig/src/main.zig

Lines 6023 to 6027 in 128fd7d

	return switch (color) {
	.auto => std.io.tty.detectConfig(std.io.getStdErr()),
	.on => .escape_codes,
	.off => .no_color,
	};

[dweiller]

I'll be happy with that. You won't be able to force the windows specific way of colouring, but I don't know if that's something you actually ever want to do.