From 8d2fb5db7e077d1db70c3ff8f0a0660a482ac313 Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Mon, 11 Jul 2022 19:03:09 +0200 Subject: [PATCH 01/14] prep: output_buffer -> output_buffer_slice --- lib/std/Progress.zig | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index fe2f7bc795b0..8f685aa54f84 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -43,7 +43,7 @@ prev_refresh_timestamp: u64 = undefined, /// This buffer represents the maximum number of bytes written to the terminal /// with each refresh. -output_buffer: [100]u8 = undefined, +output_buffer_slice: [100]u8 = undefined, /// How many nanoseconds between writing updates to the terminal. refresh_rate_ns: u64 = 50 * std.time.ns_per_ms, @@ -204,8 +204,8 @@ fn refreshWithHeldLock(self: *Progress) void { // `columns_written` cells to the left, then clear the rest of the // line if (self.supports_ansi_escape_codes) { - end += (std.fmt.bufPrint(self.output_buffer[end..], "\x1b[{d}D", .{self.columns_written}) catch unreachable).len; - end += (std.fmt.bufPrint(self.output_buffer[end..], "\x1b[0K", .{}) catch unreachable).len; + end += (std.fmt.bufPrint(self.output_buffer_slice[end..], "\x1b[{d}D", .{self.columns_written}) catch unreachable).len; + end += (std.fmt.bufPrint(self.output_buffer_slice[end..], "\x1b[0K", .{}) catch unreachable).len; } else if (builtin.os.tag == .windows) winapi: { std.debug.assert(self.is_windows_terminal); @@ -247,7 +247,7 @@ fn refreshWithHeldLock(self: *Progress) void { unreachable; } else { // we are in a "dumb" terminal like in acme or writing to a file - self.output_buffer[end] = '\n'; + self.output_buffer_slice[end] = '\n'; end += 1; } @@ -287,7 +287,7 @@ fn refreshWithHeldLock(self: *Progress) void { } } - _ = file.write(self.output_buffer[0..end]) catch { + _ = file.write(self.output_buffer_slice[0..end]) catch { // Stop trying to write to this file once it errors. self.terminal = null; }; @@ -310,16 +310,16 @@ pub fn log(self: *Progress, comptime format: []const u8, args: anytype) void { } fn bufWrite(self: *Progress, end: *usize, comptime format: []const u8, args: anytype) void { - if (std.fmt.bufPrint(self.output_buffer[end.*..], format, args)) |written| { + if (std.fmt.bufPrint(self.output_buffer_slice[end.*..], format, args)) |written| { const amt = written.len; end.* += amt; self.columns_written += amt; } else |err| switch (err) { error.NoSpaceLeft => { - self.columns_written += self.output_buffer.len - end.*; - end.* = self.output_buffer.len; + self.columns_written += self.output_buffer_slice.len - end.*; + end.* = self.output_buffer_slice.len; const suffix = "... "; - std.mem.copy(u8, self.output_buffer[self.output_buffer.len - suffix.len ..], suffix); + std.mem.copy(u8, self.output_buffer_slice[self.output_buffer_slice.len - suffix.len ..], suffix); }, } } From 5d613ae9eb1d92217485e960607dc274227735fb Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Mon, 11 Jul 2022 19:40:37 +0200 Subject: [PATCH 02/14] fix: truncate lines accurately Currently, the code assumes a terminal width of 100. If we look at what's printed for the last test: ``` Test [1/1] test "basic functionality"... [101/100] this is a really long name designed to activate the truncation code. let's fi... ``` No, it does not really work because the relevant part here is `"[101/100] this is a really long name designed to activate the truncation code. let's fi... "`, which is 90 characters, but we expect 100 because that's the width that is assumed. The reason is that it also measures **unprintable characters** (escape sequences) at least non-Windows systems. With this commit the output is now: ``` Test [1/1] test "basic functionality"... [101/100] this is a really long name designed to activate the truncation code. let's find out if... ``` Of which `"[101/100] this is a really long name designed to activate the truncation code. let's find out if... "` is the actual output of *our* `std.Progress` (remember that `zig test` has an `std.Progress` and our test itself does). The length of that string is 100. Now the length is consistent with Windows where we don't use escape sequences. This issue was only present on non-Windows systems. --- lib/std/Progress.zig | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index 8f685aa54f84..9ac3b1997540 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -43,7 +43,8 @@ prev_refresh_timestamp: u64 = undefined, /// This buffer represents the maximum number of bytes written to the terminal /// with each refresh. -output_buffer_slice: [100]u8 = undefined, +output_buffer: [256]u8 = undefined, +output_buffer_slice: []u8 = undefined, /// How many nanoseconds between writing updates to the terminal. refresh_rate_ns: u64 = 50 * std.time.ns_per_ms, @@ -198,6 +199,9 @@ fn refreshWithHeldLock(self: *Progress) void { const file = self.terminal orelse return; + // prepare for printing unprintable characters + self.output_buffer_slice = &self.output_buffer; + var end: usize = 0; if (self.columns_written > 0) { // restore the cursor position by moving the cursor @@ -254,6 +258,13 @@ fn refreshWithHeldLock(self: *Progress) void { self.columns_written = 0; } + // from here on we will print printable characters. + // make sure the unprintable characters we printed don't affect when we truncate the line + // in `bufWrite`. + const unprintables = end; + end = 0; + self.output_buffer_slice = self.output_buffer[unprintables .. unprintables + 100]; + if (!self.done) { var need_ellipse = false; var maybe_node: ?*Node = &self.root; @@ -287,7 +298,7 @@ fn refreshWithHeldLock(self: *Progress) void { } } - _ = file.write(self.output_buffer_slice[0..end]) catch { + _ = file.write(self.output_buffer[0 .. end + unprintables]) catch { // Stop trying to write to this file once it errors. self.terminal = null; }; @@ -316,6 +327,9 @@ fn bufWrite(self: *Progress, end: *usize, comptime format: []const u8, args: any self.columns_written += amt; } else |err| switch (err) { error.NoSpaceLeft => { + // truncate the line with a suffix. + // for example if we have "hello world" (len=11) and 10 is the limit, + // it would become "hello w... " self.columns_written += self.output_buffer_slice.len - end.*; end.* = self.output_buffer_slice.len; const suffix = "... "; From e2e425f0f2d5fdbf398c20dbf2d4b9335516c15b Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Mon, 11 Jul 2022 20:30:12 +0200 Subject: [PATCH 03/14] feat: decide optimal maximum width This is done by 1. getting the current terminal width and 2. subtracting that by the current cursor column. This accounts for previous output from someone else. --- lib/std/Progress.zig | 79 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 3 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index 9ac3b1997540..6fa85655435a 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -11,6 +11,7 @@ const builtin = @import("builtin"); const windows = std.os.windows; const testing = std.testing; const assert = std.debug.assert; +const os = std.os; const Progress = @This(); /// `null` if the current node (and its children) should @@ -46,6 +47,12 @@ prev_refresh_timestamp: u64 = undefined, output_buffer: [256]u8 = undefined, output_buffer_slice: []u8 = undefined, +/// This is the maximum number of bytes written to the terminal with each refresh. +/// +/// It is recommended to leave this as `null` so that `start` can automatically decide an +/// optimal width for the terminal. +max_width: ?usize = null, + /// How many nanoseconds between writing updates to the terminal. refresh_rate_ns: u64 = 50 * std.time.ns_per_ms, @@ -63,6 +70,8 @@ update_mutex: std.Thread.Mutex = .{}, /// we can move the cursor back later. columns_written: usize = undefined, +const truncation_suffix = "... "; + /// Represents one unit of progress. Each node can have children nodes, or /// one can use integers with `update`. pub const Node = struct { @@ -156,6 +165,20 @@ pub fn start(self: *Progress, name: []const u8, estimated_total_items: usize) *N // we are in a "dumb" terminal like in acme or writing to a file self.terminal = stderr; } + if (self.max_width == null) { + if (self.terminal) |terminal| { + const terminal_width = self.getTerminalWidth(terminal.handle) catch 100; + const chars_already_printed = self.getTerminalCursorColumn(terminal) catch 0; + self.max_width = terminal_width - chars_already_printed; + } else { + self.max_width = 100; + } + } + self.max_width = std.math.clamp( + self.max_width.?, + truncation_suffix.len, // make sure we can at least truncate + self.output_buffer.len, + ); self.root = Node{ .context = self, .parent = null, @@ -170,6 +193,53 @@ pub fn start(self: *Progress, name: []const u8, estimated_total_items: usize) *N return &self.root; } +fn getTerminalWidth(self: Progress, file_handle: os.fd_t) !u16 { + if (builtin.os.tag == .windows) { + std.debug.assert(self.is_windows_terminal); + var info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined; + if (windows.kernel32.GetConsoleScreenBufferInfo(file_handle, &info) != windows.TRUE) + unreachable; + return @intCast(u16, info.dwSize.X); + } else { + var winsize: os.linux.winsize = undefined; + switch (os.errno(os.linux.ioctl(file_handle, os.linux.T.IOCGWINSZ, @ptrToInt(&winsize)))) { + .SUCCESS => return winsize.ws_col, + else => return error.Unexpected, + } + } +} + +fn getTerminalCursorColumn(self: Progress, file: std.fs.File) !u16 { + if (self.supports_ansi_escape_codes) { + // First, disable echo and enable non-canonical mode + // (so that no enter press required for us to read the output of the escape sequence below) + const original_termios = try os.tcgetattr(file.handle); + var new_termios = original_termios; + new_termios.lflag &= ~(os.linux.ECHO | os.linux.ICANON); + try os.tcsetattr(file.handle, .NOW, new_termios); + defer os.tcsetattr(file.handle, .NOW, original_termios) catch { + // Sorry for ruining your terminal + }; + + try file.writeAll("\x1b[6n"); + var buf: ["\x1b[256;256R".len]u8 = undefined; + const output = try file.reader().readUntilDelimiter(&buf, 'R'); + var splitter = std.mem.split(u8, output, ";"); + _ = splitter.next().?; // skip first half + const column_half = splitter.next() orelse return error.UnexpectedEnd; + const column = try std.fmt.parseUnsigned(u16, column_half, 10); + return column - 1; // it's one-based + } else if (builtin.os.tag == .windows) { + std.debug.assert(self.is_windows_terminal); + var info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined; + if (windows.kernel32.GetConsoleScreenBufferInfo(file.handle, &info) != windows.TRUE) + unreachable; + return info.dwCursorPosition.X; + } else { + return error.Unsupported; + } +} + /// Updates the terminal if enough time has passed since last update. Thread-safe. pub fn maybeRefresh(self: *Progress) void { if (self.timer) |*timer| { @@ -263,7 +333,7 @@ fn refreshWithHeldLock(self: *Progress) void { // in `bufWrite`. const unprintables = end; end = 0; - self.output_buffer_slice = self.output_buffer[unprintables .. unprintables + 100]; + self.output_buffer_slice = self.output_buffer[unprintables .. unprintables + self.max_width.?]; if (!self.done) { var need_ellipse = false; @@ -332,8 +402,11 @@ fn bufWrite(self: *Progress, end: *usize, comptime format: []const u8, args: any // it would become "hello w... " self.columns_written += self.output_buffer_slice.len - end.*; end.* = self.output_buffer_slice.len; - const suffix = "... "; - std.mem.copy(u8, self.output_buffer_slice[self.output_buffer_slice.len - suffix.len ..], suffix); + std.mem.copy( + u8, + self.output_buffer_slice[self.output_buffer_slice.len - truncation_suffix.len ..], + truncation_suffix, + ); }, } } From 4e9ddfc1c243b7e19d377bc62b1d1010e7f32e5b Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Mon, 11 Jul 2022 20:32:02 +0200 Subject: [PATCH 04/14] test: add more tests They make it easier to see how the progress line is printed in different cases. --- lib/std/Progress.zig | 62 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 5 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index 6fa85655435a..ad93ddbc2cf3 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -411,13 +411,65 @@ fn bufWrite(self: *Progress, end: *usize, comptime format: []const u8, args: any } } -test "basic functionality" { - var disable = true; - if (disable) { - // This test is disabled because it uses time.sleep() and is therefore slow. It also - // prints bogus progress data to stderr. +// By default these tests are disabled because they use time.sleep() +// and are therefore slow. They also prints bogus progress data to stderr. +const skip_tests = false; + +test "behavior on buffer overflow" { + if (skip_tests) return error.SkipZigTest; + + var progress = Progress{}; + + const long_string = "A" ** 300; + var node = progress.start(long_string, 0); + + const speed_factor = std.time.ns_per_s / 4; + + std.time.sleep(speed_factor); + node.activate(); + std.time.sleep(speed_factor); + node.end(); +} + +test "multiple tasks with long names" { + if (skip_tests) + return error.SkipZigTest; + + const time = std.time; + + var progress = Progress{}; + + const tasks = [_][]const u8{ + "A" ** 99, + "A" ** 100, + "A" ** 101, + "A" ** 102, + "A" ** 103, + }; + + const speed_factor = time.ns_per_s / 6; + + for (tasks) |task| { + var node = progress.start(task, 3); + time.sleep(speed_factor); + node.activate(); + + time.sleep(speed_factor); + node.completeOne(); + time.sleep(speed_factor); + node.completeOne(); + time.sleep(speed_factor); + node.completeOne(); + + node.end(); } +} + +test "basic functionality" { + if (skip_tests) + return error.SkipZigTest; + var progress = Progress{}; const root_node = progress.start("", 100); defer root_node.end(); From b3fbdbcf8a74e9676997f24171527bd370d4af94 Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Mon, 11 Jul 2022 20:34:13 +0200 Subject: [PATCH 05/14] style: fix typo and improve docs It also expands an acronym used as a variable name. It confused me. --- lib/std/Progress.zig | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index ad93ddbc2cf3..87dfce6eb149 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -1,10 +1,15 @@ -//! This API non-allocating, non-fallible, and thread-safe. +//! This is a non-allocating, non-fallible, and thread-safe API for printing +//! progress indicators to the terminal. //! The tradeoff is that users of this API must provide the storage //! for each `Progress.Node`. //! +//! This library purposefully keeps its output simple and is ASCII-compatible. +//! //! Initialize the struct directly, overriding these fields as desired: //! * `refresh_rate_ms` //! * `initial_delay_ms` +//! * `dont_print_on_dumb` +//! * `max_width` const std = @import("std"); const builtin = @import("builtin"); @@ -18,7 +23,7 @@ const Progress = @This(); /// not print on update() terminal: ?std.fs.File = undefined, -/// Is this a windows API terminal (note: this is not the same as being run on windows +/// Is this a Windows API terminal (note: this is not the same as being run on Windows /// because other terminals exist like MSYS/git-bash) is_windows_terminal: bool = false, @@ -56,7 +61,7 @@ max_width: ?usize = null, /// How many nanoseconds between writing updates to the terminal. refresh_rate_ns: u64 = 50 * std.time.ns_per_ms, -/// How many nanoseconds to keep the output hidden +/// How many nanoseconds to keep the output hidden. initial_delay_ns: u64 = 500 * std.time.ns_per_ms, done: bool = true, @@ -77,6 +82,7 @@ const truncation_suffix = "... "; pub const Node = struct { context: *Progress, parent: ?*Node, + /// The name that will be displayed for this node. name: []const u8, /// Must be handled atomically to be thread-safe. recently_updated_child: ?*Node = null, @@ -167,6 +173,8 @@ pub fn start(self: *Progress, name: []const u8, estimated_total_items: usize) *N } if (self.max_width == null) { if (self.terminal) |terminal| { + // choose an optimal width and account for progress output that could have been printed + // before us by another `std.Progress` instance const terminal_width = self.getTerminalWidth(terminal.handle) catch 100; const chars_already_printed = self.getTerminalCursorColumn(terminal) catch 0; self.max_width = terminal_width - chars_already_printed; @@ -336,34 +344,34 @@ fn refreshWithHeldLock(self: *Progress) void { self.output_buffer_slice = self.output_buffer[unprintables .. unprintables + self.max_width.?]; if (!self.done) { - var need_ellipse = false; + var need_ellipsis = false; var maybe_node: ?*Node = &self.root; while (maybe_node) |node| { - if (need_ellipse) { + if (need_ellipsis) { self.bufWrite(&end, "... ", .{}); } - need_ellipse = false; - const eti = @atomicLoad(usize, &node.unprotected_estimated_total_items, .Monotonic); + need_ellipsis = false; + const estimated_total_items = @atomicLoad(usize, &node.unprotected_estimated_total_items, .Monotonic); const completed_items = @atomicLoad(usize, &node.unprotected_completed_items, .Monotonic); const current_item = completed_items + 1; - if (node.name.len != 0 or eti > 0) { + if (node.name.len != 0 or estimated_total_items > 0) { if (node.name.len != 0) { self.bufWrite(&end, "{s}", .{node.name}); - need_ellipse = true; + need_ellipsis = true; } - if (eti > 0) { - if (need_ellipse) self.bufWrite(&end, " ", .{}); - self.bufWrite(&end, "[{d}/{d}] ", .{ current_item, eti }); - need_ellipse = false; + if (estimated_total_items > 0) { + if (need_ellipsis) self.bufWrite(&end, " ", .{}); + self.bufWrite(&end, "[{d}/{d}] ", .{ current_item, estimated_total_items }); + need_ellipsis = false; } else if (completed_items != 0) { - if (need_ellipse) self.bufWrite(&end, " ", .{}); + if (need_ellipsis) self.bufWrite(&end, " ", .{}); self.bufWrite(&end, "[{d}] ", .{current_item}); - need_ellipse = false; + need_ellipsis = false; } } maybe_node = @atomicLoad(?*Node, &node.recently_updated_child, .Acquire); } - if (need_ellipse) { + if (need_ellipsis) { self.bufWrite(&end, "... ", .{}); } } From 9f7d6d8ef171d77b3ac7bb0816521b4fb00f8d68 Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Mon, 11 Jul 2022 21:06:24 +0200 Subject: [PATCH 06/14] cleanup: import std.time --- lib/std/Progress.zig | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index 87dfce6eb149..c29583bc2ce8 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -17,6 +17,7 @@ const windows = std.os.windows; const testing = std.testing; const assert = std.debug.assert; const os = std.os; +const time = std.time; const Progress = @This(); /// `null` if the current node (and its children) should @@ -41,7 +42,7 @@ root: Node = undefined, /// Keeps track of how much time has passed since the beginning. /// Used to compare with `initial_delay_ms` and `refresh_rate_ms`. -timer: ?std.time.Timer = null, +timer: ?time.Timer = null, /// When the previous refresh was written to the terminal. /// Used to compare with `refresh_rate_ms`. @@ -59,10 +60,10 @@ output_buffer_slice: []u8 = undefined, max_width: ?usize = null, /// How many nanoseconds between writing updates to the terminal. -refresh_rate_ns: u64 = 50 * std.time.ns_per_ms, +refresh_rate_ns: u64 = 50 * time.ns_per_ms, /// How many nanoseconds to keep the output hidden. -initial_delay_ns: u64 = 500 * std.time.ns_per_ms, +initial_delay_ns: u64 = 500 * time.ns_per_ms, done: bool = true, @@ -196,7 +197,7 @@ pub fn start(self: *Progress, name: []const u8, estimated_total_items: usize) *N }; self.columns_written = 0; self.prev_refresh_timestamp = 0; - self.timer = std.time.Timer.start() catch null; + self.timer = time.Timer.start() catch null; self.done = false; return &self.root; } @@ -432,11 +433,11 @@ test "behavior on buffer overflow" { const long_string = "A" ** 300; var node = progress.start(long_string, 0); - const speed_factor = std.time.ns_per_s / 4; + const speed_factor = time.ns_per_s / 4; - std.time.sleep(speed_factor); + time.sleep(speed_factor); node.activate(); - std.time.sleep(speed_factor); + time.sleep(speed_factor); node.end(); } @@ -444,8 +445,6 @@ test "multiple tasks with long names" { if (skip_tests) return error.SkipZigTest; - const time = std.time; - var progress = Progress{}; const tasks = [_][]const u8{ @@ -482,7 +481,7 @@ test "basic functionality" { const root_node = progress.start("", 100); defer root_node.end(); - const speed_factor = std.time.ns_per_ms; + const speed_factor = time.ns_per_ms; const sub_task_names = [_][]const u8{ "reticulating splines", @@ -499,24 +498,24 @@ test "basic functionality" { next_sub_task = (next_sub_task + 1) % sub_task_names.len; node.completeOne(); - std.time.sleep(5 * speed_factor); + time.sleep(5 * speed_factor); node.completeOne(); node.completeOne(); - std.time.sleep(5 * speed_factor); + time.sleep(5 * speed_factor); node.completeOne(); node.completeOne(); - std.time.sleep(5 * speed_factor); + time.sleep(5 * speed_factor); node.end(); - std.time.sleep(5 * speed_factor); + time.sleep(5 * speed_factor); } { var node = root_node.start("this is a really long name designed to activate the truncation code. let's find out if it works", 0); node.activate(); - std.time.sleep(10 * speed_factor); + time.sleep(10 * speed_factor); progress.refresh(); - std.time.sleep(10 * speed_factor); + time.sleep(10 * speed_factor); node.end(); } } From a67b1b743ea1a37b86ec676e45ef3059e066ce8c Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Mon, 11 Jul 2022 21:06:34 +0200 Subject: [PATCH 07/14] test: add test --- lib/std/Progress.zig | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index c29583bc2ce8..7b43f90e8cb5 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -473,6 +473,28 @@ test "multiple tasks with long names" { } } +test "very short max width" { + if (skip_tests) + return error.SkipZigTest; + + var progress = Progress{ .max_width = 4 }; + + const task = "A" ** 300; + + const speed_factor = time.ns_per_s / 2; + + var node = progress.start(task, 3); + time.sleep(speed_factor); + node.activate(); + + time.sleep(speed_factor); + node.completeOne(); + time.sleep(speed_factor); + node.completeOne(); + + node.end(); +} + test "basic functionality" { if (skip_tests) return error.SkipZigTest; From 14cc47ed824aed7d4d35b7a9067521a9e2defd56 Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Tue, 12 Jul 2022 18:30:29 +0200 Subject: [PATCH 08/14] fix: limit termios usage to Linux only for now --- lib/std/Progress.zig | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index 7b43f90e8cb5..f9c971d55fc3 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -209,17 +209,23 @@ fn getTerminalWidth(self: Progress, file_handle: os.fd_t) !u16 { if (windows.kernel32.GetConsoleScreenBufferInfo(file_handle, &info) != windows.TRUE) unreachable; return @intCast(u16, info.dwSize.X); - } else { + } else if (builtin.os.tag == .linux) { + // TODO: figure out how to get this working on FreeBSD, macOS etc. too. + // they too should have capabilities to figure out the cursor column. var winsize: os.linux.winsize = undefined; switch (os.errno(os.linux.ioctl(file_handle, os.linux.T.IOCGWINSZ, @ptrToInt(&winsize)))) { .SUCCESS => return winsize.ws_col, else => return error.Unexpected, } + } else { + return error.Unsupported; } } fn getTerminalCursorColumn(self: Progress, file: std.fs.File) !u16 { - if (self.supports_ansi_escape_codes) { + // TODO: figure out how to get this working on FreeBSD, macOS etc. too. + // they too should have termios or capabilities to figure out the terminal width. + if (builtin.os.tag == .linux and self.supports_ansi_escape_codes) { // First, disable echo and enable non-canonical mode // (so that no enter press required for us to read the output of the escape sequence below) const original_termios = try os.tcgetattr(file.handle); From 4fedc102d3ccf88412f622c44518d4c10a6470a2 Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Tue, 12 Jul 2022 19:18:03 +0200 Subject: [PATCH 09/14] fix: missing cast on Windows --- lib/std/Progress.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index f9c971d55fc3..a5ce7da4d44f 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -249,7 +249,7 @@ fn getTerminalCursorColumn(self: Progress, file: std.fs.File) !u16 { var info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined; if (windows.kernel32.GetConsoleScreenBufferInfo(file.handle, &info) != windows.TRUE) unreachable; - return info.dwCursorPosition.X; + return @intCast(u16, info.dwCursorPosition.X); } else { return error.Unsupported; } From 17503a5ae36003fda8c05fc3d8ff2606a432d743 Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Wed, 13 Jul 2022 20:28:16 +0200 Subject: [PATCH 10/14] test: try to debug failure --- lib/std/Progress.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index a5ce7da4d44f..53b1dacd35ae 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -348,6 +348,7 @@ fn refreshWithHeldLock(self: *Progress) void { // in `bufWrite`. const unprintables = end; end = 0; + std.debug.print("PROGRESS DEBUG: {d} {d}", .{ unprintables, self.max_width.? }); self.output_buffer_slice = self.output_buffer[unprintables .. unprintables + self.max_width.?]; if (!self.done) { From 5fd2c45ee5f9f750b9a8f93f65bd354a42f83913 Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Thu, 14 Jul 2022 08:43:33 +0200 Subject: [PATCH 11/14] fix: fix off-by-one and disable tests --- lib/std/Progress.zig | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index 53b1dacd35ae..f96224fa897f 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -186,7 +186,7 @@ pub fn start(self: *Progress, name: []const u8, estimated_total_items: usize) *N self.max_width = std.math.clamp( self.max_width.?, truncation_suffix.len, // make sure we can at least truncate - self.output_buffer.len, + self.output_buffer.len - 1, ); self.root = Node{ .context = self, @@ -348,7 +348,6 @@ fn refreshWithHeldLock(self: *Progress) void { // in `bufWrite`. const unprintables = end; end = 0; - std.debug.print("PROGRESS DEBUG: {d} {d}", .{ unprintables, self.max_width.? }); self.output_buffer_slice = self.output_buffer[unprintables .. unprintables + self.max_width.?]; if (!self.done) { @@ -429,12 +428,15 @@ fn bufWrite(self: *Progress, end: *usize, comptime format: []const u8, args: any // By default these tests are disabled because they use time.sleep() // and are therefore slow. They also prints bogus progress data to stderr. -const skip_tests = false; +const skip_tests = true; test "behavior on buffer overflow" { if (skip_tests) return error.SkipZigTest; + // move the cursor + std.debug.print("{s}", .{"A" ** 300}); + var progress = Progress{}; const long_string = "A" ** 300; From 4f9149128d010ed408b49c386f7d2d4f231fa21c Mon Sep 17 00:00:00 2001 From: r00ster Date: Sun, 24 Jul 2022 21:51:30 +0200 Subject: [PATCH 12/14] docs: make comment clearer --- lib/std/Progress.zig | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index f96224fa897f..5e777c3e17d8 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -290,8 +290,7 @@ fn refreshWithHeldLock(self: *Progress) void { var end: usize = 0; if (self.columns_written > 0) { // restore the cursor position by moving the cursor - // `columns_written` cells to the left, then clear the rest of the - // line + // `columns_written` cells to the left, then clear the rest of the line if (self.supports_ansi_escape_codes) { end += (std.fmt.bufPrint(self.output_buffer_slice[end..], "\x1b[{d}D", .{self.columns_written}) catch unreachable).len; end += (std.fmt.bufPrint(self.output_buffer_slice[end..], "\x1b[0K", .{}) catch unreachable).len; @@ -343,9 +342,8 @@ fn refreshWithHeldLock(self: *Progress) void { self.columns_written = 0; } - // from here on we will print printable characters. - // make sure the unprintable characters we printed don't affect when we truncate the line - // in `bufWrite`. + // from here on we will write printable characters. we also make sure the unprintable characters + // we possibly wrote previously don't affect whether we truncate the line in `bufWrite`. const unprintables = end; end = 0; self.output_buffer_slice = self.output_buffer[unprintables .. unprintables + self.max_width.?]; From cbdb29db12cf4778d746888dc8713c4c61de8744 Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Sat, 30 Jul 2022 14:28:05 +0200 Subject: [PATCH 13/14] fix: more durability --- lib/std/Progress.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index 5e777c3e17d8..8bb85c08ff85 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -207,7 +207,7 @@ fn getTerminalWidth(self: Progress, file_handle: os.fd_t) !u16 { std.debug.assert(self.is_windows_terminal); var info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined; if (windows.kernel32.GetConsoleScreenBufferInfo(file_handle, &info) != windows.TRUE) - unreachable; + return error.Unexpected; return @intCast(u16, info.dwSize.X); } else if (builtin.os.tag == .linux) { // TODO: figure out how to get this working on FreeBSD, macOS etc. too. @@ -248,7 +248,7 @@ fn getTerminalCursorColumn(self: Progress, file: std.fs.File) !u16 { std.debug.assert(self.is_windows_terminal); var info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined; if (windows.kernel32.GetConsoleScreenBufferInfo(file.handle, &info) != windows.TRUE) - unreachable; + return error.Unexpected; return @intCast(u16, info.dwCursorPosition.X); } else { return error.Unsupported; From fe49b025d318345581c1a7214e20b4ce5b6514d9 Mon Sep 17 00:00:00 2001 From: r00ster91 Date: Thu, 29 Sep 2022 18:06:11 +0200 Subject: [PATCH 14/14] fix(getTerminalWidth): change order --- lib/std/Progress.zig | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index 8bb85c08ff85..d077bb7002de 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -203,13 +203,7 @@ pub fn start(self: *Progress, name: []const u8, estimated_total_items: usize) *N } fn getTerminalWidth(self: Progress, file_handle: os.fd_t) !u16 { - if (builtin.os.tag == .windows) { - std.debug.assert(self.is_windows_terminal); - var info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined; - if (windows.kernel32.GetConsoleScreenBufferInfo(file_handle, &info) != windows.TRUE) - return error.Unexpected; - return @intCast(u16, info.dwSize.X); - } else if (builtin.os.tag == .linux) { + if (builtin.os.tag == .linux) { // TODO: figure out how to get this working on FreeBSD, macOS etc. too. // they too should have capabilities to figure out the cursor column. var winsize: os.linux.winsize = undefined; @@ -217,6 +211,12 @@ fn getTerminalWidth(self: Progress, file_handle: os.fd_t) !u16 { .SUCCESS => return winsize.ws_col, else => return error.Unexpected, } + } else if (builtin.os.tag == .windows) { + std.debug.assert(self.is_windows_terminal); + var info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined; + if (windows.kernel32.GetConsoleScreenBufferInfo(file_handle, &info) != windows.TRUE) + return error.Unexpected; + return @intCast(u16, info.dwSize.X); } else { return error.Unsupported; }