Commit 88d927a511

Ryan Liptak <squeek502@hotmail.com>
2022-12-15 05:01:11
std.debug.TTY: Fix colors not resetting on Windows
This fixes a regression introduced in #12298 where colors would never reset in a Windows console because the attributes would be queried on every `setColor` call, and then try to 'reset' the attributes to what it just queried (i.e. it was essentially doing a complicated no-op on .Reset). This fixes the problem while (I think) keeping with the spirit of the changes in #12298--that is, `TTY.Config` is not specifically tied to stderr like it was before #12298. To that end, detectTTYConfig now takes a `File` and that's what gets used to query the initial attributes to reset to. (for context, before #12298, the first `setColor` call is where the reset attributes would get queried and it would always use stderr to do it)
master
1 parent 83e0e23
Changed files (4)
lib
std
builtin.zig
debug.zig
testing.zig
src
main.zig
lib/std/builtin.zig
@@ -51,7 +51,7 @@ pub const StackTrace = struct {
         const debug_info = std.debug.getSelfDebugInfo() catch |err| {
             return writer.print("\nUnable to print stack trace: Unable to open debug info: {s}\n", .{@errorName(err)});
         };
-        const tty_config = std.debug.detectTTYConfig();
+        const tty_config = std.debug.detectTTYConfig(std.io.getStdErr());
         try writer.writeAll("\n");
         std.debug.writeStackTrace(self, writer, arena.allocator(), debug_info, tty_config) catch |err| {
             try writer.print("Unable to print stack trace: {s}\n", .{@errorName(err)});
lib/std/debug.zig
@@ -109,17 +109,24 @@ pub fn getSelfDebugInfo() !*DebugInfo {
     }
 }
 
-pub fn detectTTYConfig() TTY.Config {
+pub fn detectTTYConfig(file: std.fs.File) TTY.Config {
     if (process.hasEnvVarConstant("ZIG_DEBUG_COLOR")) {
         return .escape_codes;
     } else if (process.hasEnvVarConstant("NO_COLOR")) {
         return .no_color;
     } else {
-        const stderr_file = io.getStdErr();
-        if (stderr_file.supportsAnsiEscapeCodes()) {
+        if (file.supportsAnsiEscapeCodes()) {
             return .escape_codes;
-        } else if (native_os == .windows and stderr_file.isTty()) {
-            return .{ .windows_api = stderr_file };
+        } else if (native_os == .windows and file.isTty()) {
+            var info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined;
+            if (windows.kernel32.GetConsoleScreenBufferInfo(file.handle, &info) != windows.TRUE) {
+                // TODO: Should this return an error instead?
+                return .no_color;
+            }
+            return .{ .windows_api = .{
+                .handle = file.handle,
+                .reset_attributes = info.wAttributes,
+            } };
         } else {
             return .no_color;
         }
@@ -146,7 +153,7 @@ pub fn dumpCurrentStackTrace(start_addr: ?usize) void {
             stderr.print("Unable to dump stack trace: Unable to open debug info: {s}\n", .{@errorName(err)}) catch return;
             return;
         };
-        writeCurrentStackTrace(stderr, debug_info, detectTTYConfig(), start_addr) catch |err| {
+        writeCurrentStackTrace(stderr, debug_info, detectTTYConfig(io.getStdErr()), start_addr) catch |err| {
             stderr.print("Unable to dump stack trace: {s}\n", .{@errorName(err)}) catch return;
             return;
         };
@@ -174,7 +181,7 @@ pub fn dumpStackTraceFromBase(bp: usize, ip: usize) void {
             stderr.print("Unable to dump stack trace: Unable to open debug info: {s}\n", .{@errorName(err)}) catch return;
             return;
         };
-        const tty_config = detectTTYConfig();
+        const tty_config = detectTTYConfig(io.getStdErr());
         printSourceAtAddress(debug_info, stderr, ip, tty_config) catch return;
         var it = StackIterator.init(null, bp);
         while (it.next()) |return_address| {
@@ -257,7 +264,7 @@ pub fn dumpStackTrace(stack_trace: std.builtin.StackTrace) void {
             stderr.print("Unable to dump stack trace: Unable to open debug info: {s}\n", .{@errorName(err)}) catch return;
             return;
         };
-        writeStackTrace(stack_trace, stderr, getDebugInfoAllocator(), debug_info, detectTTYConfig()) catch |err| {
+        writeStackTrace(stack_trace, stderr, getDebugInfoAllocator(), debug_info, detectTTYConfig(io.getStdErr())) catch |err| {
             stderr.print("Unable to dump stack trace: {s}\n", .{@errorName(err)}) catch return;
             return;
         };
@@ -600,7 +607,12 @@ pub const TTY = struct {
     pub const Config = union(enum) {
         no_color,
         escape_codes,
-        windows_api: File,
+        windows_api: if (native_os == .windows) WindowsContext else void,
+
+        pub const WindowsContext = struct {
+            handle: File.Handle,
+            reset_attributes: u16,
+        };
 
         pub fn setColor(conf: Config, out_stream: anytype, color: Color) !void {
             nosuspend switch (conf) {
@@ -617,19 +629,16 @@ pub const TTY = struct {
                     };
                     try out_stream.writeAll(color_string);
                 },
-                .windows_api => |file| if (native_os == .windows) {
-                    var info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined;
-                    if (windows.kernel32.GetConsoleScreenBufferInfo(file.handle, &info) != windows.TRUE)
-                        return error.FailedRetrievingTerminalInfo;
+                .windows_api => |ctx| if (native_os == .windows) {
                     const attributes = switch (color) {
                         .Red => windows.FOREGROUND_RED | windows.FOREGROUND_INTENSITY,
                         .Green => windows.FOREGROUND_GREEN | windows.FOREGROUND_INTENSITY,
                         .Cyan => windows.FOREGROUND_GREEN | windows.FOREGROUND_BLUE | windows.FOREGROUND_INTENSITY,
                         .White, .Bold => windows.FOREGROUND_RED | windows.FOREGROUND_GREEN | windows.FOREGROUND_BLUE | windows.FOREGROUND_INTENSITY,
                         .Dim => windows.FOREGROUND_INTENSITY,
-                        .Reset => info.wAttributes,
+                        .Reset => ctx.reset_attributes,
                     };
-                    try windows.SetConsoleTextAttribute(file.handle, attributes);
+                    try windows.SetConsoleTextAttribute(ctx.handle, attributes);
                 } else {
                     unreachable;
                 },
@@ -2005,7 +2014,7 @@ test "#4353: std.debug should manage resources correctly" {
     const writer = std.io.null_writer;
     var di = try openSelfDebugInfo(testing.allocator);
     defer di.deinit();
-    try printSourceAtAddress(&di, writer, showMyTrace(), detectTTYConfig());
+    try printSourceAtAddress(&di, writer, showMyTrace(), detectTTYConfig(std.io.getStdErr()));
 }
 
 noinline fn showMyTrace() usize {
@@ -2065,7 +2074,7 @@ pub fn ConfigurableTrace(comptime size: usize, comptime stack_frame_count: usize
         pub fn dump(t: @This()) void {
             if (!enabled) return;
 
-            const tty_config = detectTTYConfig();
+            const tty_config = detectTTYConfig(std.io.getStdErr());
             const stderr = io.getStdErr().writer();
             const end = @min(t.index, size);
             const debug_info = getSelfDebugInfo() catch |err| {
lib/std/testing.zig
@@ -312,7 +312,7 @@ pub fn expectEqualSlices(comptime T: type, expected: []const T, actual: []const
     const actual_window = actual[window_start..@min(actual.len, window_start + max_window_size)];
     const actual_truncated = window_start + actual_window.len < actual.len;
 
-    const ttyconf = std.debug.detectTTYConfig();
+    const ttyconf = std.debug.detectTTYConfig(std.io.getStdErr());
     var differ = if (T == u8) BytesDiffer{
         .expected = expected_window,
         .actual = actual_window,
src/main.zig
@@ -3431,7 +3431,7 @@ fn updateModule(gpa: Allocator, comp: *Compilation, hook: AfterUpdateHook) !void
 
     if (errors.list.len != 0) {
         const ttyconf: std.debug.TTY.Config = switch (comp.color) {
-            .auto => std.debug.detectTTYConfig(),
+            .auto => std.debug.detectTTYConfig(std.io.getStdErr()),
             .on => .escape_codes,
             .off => .no_color,
         };
@@ -4252,7 +4252,7 @@ pub fn cmdFmt(gpa: Allocator, arena: Allocator, args: []const []const u8) !void
 
                 try Compilation.AllErrors.addZir(arena_instance.allocator(), &errors, &file);
                 const ttyconf: std.debug.TTY.Config = switch (color) {
-                    .auto => std.debug.detectTTYConfig(),
+                    .auto => std.debug.detectTTYConfig(std.io.getStdErr()),
                     .on => .escape_codes,
                     .off => .no_color,
                 };
@@ -4465,7 +4465,7 @@ fn fmtPathFile(
 
             try Compilation.AllErrors.addZir(arena_instance.allocator(), &errors, &file);
             const ttyconf: std.debug.TTY.Config = switch (fmt.color) {
-                .auto => std.debug.detectTTYConfig(),
+                .auto => std.debug.detectTTYConfig(std.io.getStdErr()),
                 .on => .escape_codes,
                 .off => .no_color,
             };
@@ -4586,7 +4586,7 @@ fn printErrsMsgToStdErr(
         };
 
         const ttyconf: std.debug.TTY.Config = switch (color) {
-            .auto => std.debug.detectTTYConfig(),
+            .auto => std.debug.detectTTYConfig(std.io.getStdErr()),
             .on => .escape_codes,
             .off => .no_color,
         };
@@ -5176,7 +5176,7 @@ pub fn cmdAstCheck(
         var errors = std.ArrayList(Compilation.AllErrors.Message).init(arena);
         try Compilation.AllErrors.addZir(arena, &errors, &file);
         const ttyconf: std.debug.TTY.Config = switch (color) {
-            .auto => std.debug.detectTTYConfig(),
+            .auto => std.debug.detectTTYConfig(std.io.getStdErr()),
             .on => .escape_codes,
             .off => .no_color,
         };
@@ -5301,7 +5301,7 @@ pub fn cmdChangelist(
     if (file.zir.hasCompileErrors()) {
         var errors = std.ArrayList(Compilation.AllErrors.Message).init(arena);
         try Compilation.AllErrors.addZir(arena, &errors, &file);
-        const ttyconf = std.debug.detectTTYConfig();
+        const ttyconf = std.debug.detectTTYConfig(std.io.getStdErr());
         for (errors.items) |full_err_msg| {
             full_err_msg.renderToStdErr(ttyconf);
         }
@@ -5340,7 +5340,7 @@ pub fn cmdChangelist(
     if (file.zir.hasCompileErrors()) {
         var errors = std.ArrayList(Compilation.AllErrors.Message).init(arena);
         try Compilation.AllErrors.addZir(arena, &errors, &file);
-        const ttyconf = std.debug.detectTTYConfig();
+        const ttyconf = std.debug.detectTTYConfig(std.io.getStdErr());
         for (errors.items) |full_err_msg| {
             full_err_msg.renderToStdErr(ttyconf);
         }