From c312572236063a422fad19007a73b123dd955eed Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 31 May 2024 11:54:44 -0700 Subject: [PATCH] std.Progress: keep the cursor at the beginning This changes the terminal display to keep the cursor at the top left of the progress display, so that unlocked stderr writes, perhaps by child processes, don't get eaten by the clear. --- lib/std/Progress.zig | 168 +++++++++++++++++++++---------------------- 1 file changed, 83 insertions(+), 85 deletions(-) diff --git a/lib/std/Progress.zig b/lib/std/Progress.zig index 1765dc982533..5db9296cdac3 100644 --- a/lib/std/Progress.zig +++ b/lib/std/Progress.zig @@ -23,17 +23,13 @@ redraw_event: std.Thread.ResetEvent, /// Indicates a request to shut down and reset global state. /// Accessed atomically. done: bool, +need_clear: bool, refresh_rate_ns: u64, initial_delay_ns: u64, rows: u16, cols: u16, -/// Tracks the number of newlines that have been actually written to the terminal. -written_newline_count: u16, -/// Tracks the number of newlines that will be written to the terminal if the -/// draw buffer is sent. -accumulated_newline_count: u16, /// Accessed only by the update thread. draw_buffer: []u8, @@ -312,10 +308,9 @@ var global_progress: Progress = .{ .initial_delay_ns = undefined, .rows = 0, .cols = 0, - .written_newline_count = 0, - .accumulated_newline_count = 0, .draw_buffer = undefined, .done = false, + .need_clear = false, .node_parents = &node_parents_buffer, .node_storage = &node_storage_buffer, @@ -446,10 +441,11 @@ fn updateThreadRun() void { if (@atomicLoad(bool, &global_progress.done, .seq_cst)) return; maybeUpdateSize(resize_flag); - const buffer = computeRedraw(&serialized_buffer); + const buffer, _ = computeRedraw(&serialized_buffer); if (stderr_mutex.tryLock()) { defer stderr_mutex.unlock(); write(buffer) catch return; + global_progress.need_clear = true; } } @@ -464,10 +460,11 @@ fn updateThreadRun() void { maybeUpdateSize(resize_flag); - const buffer = computeRedraw(&serialized_buffer); + const buffer, _ = computeRedraw(&serialized_buffer); if (stderr_mutex.tryLock()) { defer stderr_mutex.unlock(); write(buffer) catch return; + global_progress.need_clear = true; } } } @@ -488,11 +485,13 @@ fn windowsApiUpdateThreadRun() void { if (@atomicLoad(bool, &global_progress.done, .seq_cst)) return; maybeUpdateSize(resize_flag); - const buffer = computeRedraw(&serialized_buffer); + const buffer, const nl_n = computeRedraw(&serialized_buffer); if (stderr_mutex.tryLock()) { defer stderr_mutex.unlock(); windowsApiWriteMarker(); write(buffer) catch return; + global_progress.need_clear = true; + windowsApiMoveToMarker(nl_n) catch return; } } @@ -507,12 +506,14 @@ fn windowsApiUpdateThreadRun() void { maybeUpdateSize(resize_flag); - const buffer = computeRedraw(&serialized_buffer); + const buffer, const nl_n = computeRedraw(&serialized_buffer); if (stderr_mutex.tryLock()) { defer stderr_mutex.unlock(); clearWrittenWindowsApi() catch return; windowsApiWriteMarker(); write(buffer) catch return; + global_progress.need_clear = true; + windowsApiMoveToMarker(nl_n) catch return; } } } @@ -645,40 +646,16 @@ fn appendTreeSymbol(symbol: TreeSymbol, buf: []u8, start_i: usize) usize { } fn clearWrittenWithEscapeCodes() anyerror!void { - if (global_progress.written_newline_count == 0) return; + if (!global_progress.need_clear) return; var i: usize = 0; const buf = global_progress.draw_buffer; - buf[i..][0..start_sync.len].* = start_sync.*; - i += start_sync.len; - - i = computeClear(buf, i); - - buf[i..][0..finish_sync.len].* = finish_sync.*; - i += finish_sync.len; - - global_progress.accumulated_newline_count = 0; - try write(buf[0..i]); -} - -fn computeClear(buf: []u8, start_i: usize) usize { - var i = start_i; - - const prev_nl_n = global_progress.written_newline_count; - if (prev_nl_n > 0) { - buf[i] = '\r'; - i += 1; - for (0..prev_nl_n) |_| { - buf[i..][0..up_one_line.len].* = up_one_line.*; - i += up_one_line.len; - } - } - buf[i..][0..clear.len].* = clear.*; i += clear.len; - return i; + global_progress.need_clear = false; + try write(buf[0..i]); } /// U+25BA or ► @@ -704,38 +681,44 @@ fn clearWrittenWindowsApi() error{Unexpected}!void { // but it must be a valid attribute and it actually needs to apply to the first // character in order to be readable via ReadConsoleOutputAttribute. It doesn't seem // like any of the available attributes are invisible/benign. - const prev_nl_n = global_progress.written_newline_count; - if (prev_nl_n > 0) { - const handle = global_progress.terminal.handle; - const screen_area = @as(windows.DWORD, global_progress.cols) * global_progress.rows; + if (!global_progress.need_clear) return; + const handle = global_progress.terminal.handle; + const screen_area = @as(windows.DWORD, global_progress.cols) * global_progress.rows; - var console_info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined; - if (windows.kernel32.GetConsoleScreenBufferInfo(handle, &console_info) == 0) { - return error.Unexpected; - } - const cursor_pos = console_info.dwCursorPosition; - const expected_y = cursor_pos.Y - @as(i16, @intCast(prev_nl_n)); - var start_pos = windows.COORD{ .X = 0, .Y = expected_y }; - while (start_pos.Y >= 0) { - var wchar: [1]u16 = undefined; - var num_console_chars_read: windows.DWORD = undefined; - if (windows.kernel32.ReadConsoleOutputCharacterW(handle, &wchar, wchar.len, start_pos, &num_console_chars_read) == 0) { - return error.Unexpected; - } + var console_info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined; + if (windows.kernel32.GetConsoleScreenBufferInfo(handle, &console_info) == 0) { + return error.Unexpected; + } + var num_chars_written: windows.DWORD = undefined; + if (windows.kernel32.FillConsoleOutputCharacterW(handle, ' ', screen_area, console_info.dwCursorPosition, &num_chars_written) == 0) { + return error.Unexpected; + } +} - if (wchar[0] == windows_api_start_marker) break; - start_pos.Y -= 1; - } else { - // If we couldn't find the marker, then just assume that no lines wrapped - start_pos = .{ .X = 0, .Y = expected_y }; - } - var num_chars_written: windows.DWORD = undefined; - if (windows.kernel32.FillConsoleOutputCharacterW(handle, ' ', screen_area, start_pos, &num_chars_written) == 0) { - return error.Unexpected; - } - if (windows.kernel32.SetConsoleCursorPosition(handle, start_pos) == 0) { +fn windowsApiMoveToMarker(nl_n: usize) error{Unexpected}!void { + const handle = global_progress.terminal.handle; + var console_info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined; + if (windows.kernel32.GetConsoleScreenBufferInfo(handle, &console_info) == 0) { + return error.Unexpected; + } + const cursor_pos = console_info.dwCursorPosition; + const expected_y = cursor_pos.Y - @as(i16, @intCast(nl_n)); + var start_pos: windows.COORD = .{ .X = 0, .Y = expected_y }; + while (start_pos.Y >= 0) { + var wchar: [1]u16 = undefined; + var num_console_chars_read: windows.DWORD = undefined; + if (windows.kernel32.ReadConsoleOutputCharacterW(handle, &wchar, wchar.len, start_pos, &num_console_chars_read) == 0) { return error.Unexpected; } + + if (wchar[0] == windows_api_start_marker) break; + start_pos.Y -= 1; + } else { + // If we couldn't find the marker, then just assume that no lines wrapped + start_pos = .{ .X = 0, .Y = expected_y }; + } + if (windows.kernel32.SetConsoleCursorPosition(handle, start_pos) == 0) { + return error.Unexpected; } } @@ -1052,7 +1035,7 @@ fn useSavedIpcData( return start_serialized_len + storage.len; } -fn computeRedraw(serialized_buffer: *Serialized.Buffer) []u8 { +fn computeRedraw(serialized_buffer: *Serialized.Buffer) struct { []u8, usize } { const serialized = serialize(serialized_buffer); // Now we can analyze our copy of the graph without atomics, reconstructing @@ -1078,8 +1061,10 @@ fn computeRedraw(serialized_buffer: *Serialized.Buffer) []u8 { } } - // The strategy is: keep the cursor at the end, and then with every redraw: - // move cursor to beginning of line, move cursor up N lines, erase to end of screen, write + // The strategy is, with every redraw: + // erase to end of screen, write, move cursor to beginning of line, move cursor up N lines + // This keeps the cursor at the beginning so that unlocked stderr writes + // don't get eaten by the clear. var i: usize = 0; const buf = global_progress.draw_buffer; @@ -1091,20 +1076,31 @@ fn computeRedraw(serialized_buffer: *Serialized.Buffer) []u8 { switch (global_progress.terminal_mode) { .off => unreachable, - .ansi_escape_codes => i = computeClear(buf, i), + .ansi_escape_codes => { + buf[i..][0..clear.len].* = clear.*; + i += clear.len; + }, .windows_api => if (!is_windows) unreachable, } - global_progress.accumulated_newline_count = 0; const root_node_index: Node.Index = @enumFromInt(0); - i = computeNode(buf, i, serialized, children, root_node_index); + i, const nl_n = computeNode(buf, i, 0, serialized, children, root_node_index); if (global_progress.terminal_mode == .ansi_escape_codes) { + if (nl_n > 0) { + buf[i] = '\r'; + i += 1; + for (0..nl_n) |_| { + buf[i..][0..up_one_line.len].* = up_one_line.*; + i += up_one_line.len; + } + } + buf[i..][0..finish_sync.len].* = finish_sync.*; i += finish_sync.len; } - return buf[0..i]; + return .{ buf[0..i], nl_n }; } fn computePrefix( @@ -1138,20 +1134,23 @@ fn computePrefix( } const line_upper_bound_len = @max(TreeSymbol.tee.maxByteLen(), TreeSymbol.langle.maxByteLen()) + - "[4294967296/4294967296] ".len + Node.max_name_len + finish_sync.len; + "[4294967296/4294967296] ".len + Node.max_name_len + (1 + up_one_line.len) + finish_sync.len; fn computeNode( buf: []u8, start_i: usize, + start_nl_n: usize, serialized: Serialized, children: []const Children, node_index: Node.Index, -) usize { +) struct { usize, usize } { var i = start_i; + var nl_n = start_nl_n; + i = computePrefix(buf, i, serialized, children, node_index); if (i + line_upper_bound_len > buf.len) - return start_i; + return .{ start_i, start_nl_n }; const storage = &serialized.storage[@intFromEnum(node_index)]; const estimated_total = storage.estimated_total_count; @@ -1186,34 +1185,33 @@ fn computeNode( i = @min(global_progress.cols + start_i, i); buf[i] = '\n'; i += 1; - global_progress.accumulated_newline_count += 1; + nl_n += 1; } - if (global_progress.withinRowLimit()) { + if (global_progress.withinRowLimit(nl_n)) { if (children[@intFromEnum(node_index)].child.unwrap()) |child| { - i = computeNode(buf, i, serialized, children, child); + i, nl_n = computeNode(buf, i, nl_n, serialized, children, child); } } - if (global_progress.withinRowLimit()) { + if (global_progress.withinRowLimit(nl_n)) { if (children[@intFromEnum(node_index)].sibling.unwrap()) |sibling| { - i = computeNode(buf, i, serialized, children, sibling); + i, nl_n = computeNode(buf, i, nl_n, serialized, children, sibling); } } - return i; + return .{ i, nl_n }; } -fn withinRowLimit(p: *Progress) bool { +fn withinRowLimit(p: *Progress, nl_n: usize) bool { // The +2 here is so that the PS1 is not scrolled off the top of the terminal. // one because we keep the cursor on the next line // one more to account for the PS1 - return p.accumulated_newline_count + 2 < p.rows; + return nl_n + 2 < p.rows; } fn write(buf: []const u8) anyerror!void { try global_progress.terminal.writeAll(buf); - global_progress.written_newline_count = global_progress.accumulated_newline_count; } var remaining_write_trash_bytes: usize = 0;