Repository navigation
tty.Config.setColor is no longer unified across Escape Codes/Windows Console APIs #15976
Description
This is something of a follow-up to #15806.
Before that PR, all colors had an implicit 'bold,' and all colors in the Windows API had an implicit FOREGROUND_INTENSITY.
I've pushed an update to make colors not bold by default (the
;1m), which kind of defeated the purpose of.bold
This makes sense to do from the perspective of ANSI escape sequences, but it complicates making setColor work similarly when the Config is .windows_api. In the Windows API, there's no builtin way to persist something like .bold, so attempting to use .bold and then set a color does not have the intended effect--the 'bold' is always lost after setting the color.
About bold
Historically, 'bold' has been implemented as making the text a brighter color rather than actually bold. From Wikipedia:
Quite a few terminals implemented "bold" (SGR code 1) as a brighter color rather than a different font, thus providing 8 additional foreground colors.
This is still an option in e.g. Gnome terminal ('Show bold text in bright colors'), and is the default behavior in Windows Terminal. This doesn't really affect the problem, but it's worth noting when thinking about potential solutions.
A visualization
This is cmd.exe in Windows Terminal with the default font/settings:
(note: dim is not expected to match, there's no 'dim' equivalent in the Windows Console API)
Note that with modified settings to use a font that supports bold text and to render bold text as bold only (instead of the default setting of rendering bold text as 'bright colors'), the difference is less impactful visually but the difference can still be seen:
Code used to generate this output
const std = @import("std");
const is_windows = @import("builtin").os.tag == .windows;
pub fn main() !void {
const stderr = std.io.getStdErr();
if (is_windows) {
_ = std.os.windows.kernel32.SetConsoleOutputCP(65001);
}
const ansi_config = std.io.tty.Config{ .escape_codes = {} };
std.debug.print("ANSI Escape Codes:\n", .{});
try dumpColors(stderr.writer(), ansi_config);
if (is_windows) {
var info: std.os.windows.CONSOLE_SCREEN_BUFFER_INFO = undefined;
std.debug.assert(std.os.windows.kernel32.GetConsoleScreenBufferInfo(stderr.handle, &info) == std.os.windows.TRUE);
const console_config = std.io.tty.Config{ .windows_api = .{
.handle = stderr.handle,
.reset_attributes = info.wAttributes,
} };
std.debug.print("\nConsole API:\n", .{});
try dumpColors(stderr.writer(), console_config);
}
}
fn dumpColors(writer: anytype, config: std.io.tty.Config) !void {
inline for (@typeInfo(std.io.tty.Color).Enum.fields) |field| {
try config.setColor(writer, @field(std.io.tty.Color, field.name));
try writer.writeAll("■");
if (@field(std.io.tty.Color, field.name) == .black) {
try config.setColor(writer, .reset);
}
try writer.writeAll(" ");
try writer.writeAll(field.name);
try writer.writeByteNTimes(' ', 20 - field.name.len);
try config.setColor(writer, .bold);
try config.setColor(writer, @field(std.io.tty.Color, field.name));
try writer.writeAll("■");
if (@field(std.io.tty.Color, field.name) == .black) {
try config.setColor(writer, .reset);
try config.setColor(writer, .bold);
}
try writer.writeAll(" bold ");
try writer.writeAll(field.name);
try config.setColor(writer, .reset);
try writer.writeByteNTimes(' ', 20 - field.name.len);
try config.setColor(writer, .dim);
try config.setColor(writer, @field(std.io.tty.Color, field.name));
try writer.writeAll("■");
if (@field(std.io.tty.Color, field.name) == .black) {
try config.setColor(writer, .reset);
try config.setColor(writer, .dim);
}
try writer.writeAll(" dim ");
try writer.writeAll(field.name);
try config.setColor(writer, .reset);
try writer.writeAll("\n");
}
}Potential solutions
- Go back to every-color-is-bold/every-color-is-intense. This gives us an easy path to cross-API conformity but makes the
setColorAPI less useful/flexible than it could be. - Modify the
setColorAPI to take a mutable*Config, add something likeintensity: enum { normal, bold, dim }toConfig.WindowsContextand apply/removeFOREGROUND_INTENSITYaccordingly during eachsetColorcall. This would keep the usefulness of the API but add a wrinkle toConfigwhere the same instance would need to be re-used consistently (and without otherSetConsoleTextAttributecalls in betweensetColorcalls) in order to ensure that theintensitystate would remain correct. - Add a
GetConsoleTextAttributecall insetColor, check forFOREGROUND_INTENSITYand persist it if it's present. This would provide some support for persistentboldbut it may not always behave in the intended/expected way from the perspective of the user, and would add the overhead of an extraGetConsoleTextAttributecall. - Keep it as is and accept the incongruous behavior between the APIs. Ideally this would be supplemented by windows: detect ANSI support in more terminals #15282 getting merged which would make
.escape_codesget used on Windows in more situations (e.g. in Windows Terminal).
I personally see some merit to all of the potential solutions, and I may be missing some potential solutions.
cc @linusg if you'd like to weigh in
Activity
andrewrk commentedon Jul 23, 2023
Thanks for this writeup, @squeek502.
If you have a preference, I'd be happy to back your suggestion.
Personally, I think it was nice how it was before:
I think it's perfectly reasonable to have a minimal API that is explicitly designed to be the cross-platform common denominator. I also think it's reasonable to have an API that is intended to provide maximum flexibility on all platforms. As for which one belongs in the standard library, I could see an argument either way, but I can say for sure that the zig compiler itself wants the former.
linusg commentedon Jul 23, 2023
I saw that I got tagged in the original issue but missed it, apologies!
While I understand the desire to have equally simple and capable APIs regardless of the platform, setting a text rendering to bold for any color at the same time decreases usability of the API for environments (in this specific case, terminals) where colored text rendering is supported.
I've patched my local zig checkout to have 5d3f3ca partially reverted:
The bold+color combination is too much for my personal taste, and ultimately not something I've asked it to do (I told it "make this text colored").
So if this became the default again I'd revert to implementing my own text styling, and obviously I'd like to avoid that. I would be happy with this being the default and some way of opting out, however - even if that's just a flag in the
tty.Configthat modifies the internal behavior ofsetColor()(which can keep taking a constConfig).squeek502 commentedon Jul 23, 2023
I think it might be okay to keep the Windows Console API behaving differently, as the behavior is not too different (can basically explain it as 'bold and dim styles are not supported by the Windows Console API'), and since #16080 has been merged, Windows will use ANSI escape codes more often.
Although one thing that is a problem is that the order of setting the style and the color matters:
This is kind of an unsolvable problem with the current API. We could make
.bolddo nothing on.windows_apito avoid the issue there, but the same problem would exist with.dimand that's less solvable since.dimon its own is something that's used pretty often.Another possible way to address this would be to add an API that applies both a style and a color in one call. Something like:
I'm sure it could be improved a bit more but that'd be the idea.
andrewrk commentedon Jul 24, 2023
is this related to #16526?