Commit 4fc2e92876

LemonBoy <thatlemon@gmail.com>
2021-03-07 15:23:20
std: Better handling of line-wrapping in Progress
In order to update the printed progress string the code tried to move the cursor N cells to the left, where N is the number of written bytes, and then clear the remaining part of the line. This strategy has two main issues: - Is only valid if the number of characters is equal to the number of written bytes, - Is only valid if the line doesn't get too long. The second point is the main motivation for this change, when the line becomes too long the terminal wraps it to a new physical line. This means that moving the cursor to the left won't be enough anymore as once the left border is reached it cannot move anymore. The wrapped line is still stored by the terminal as a single line, despite now taking more than a single one when displayed. If you try to resize the terminal you'll notice how the contents are reflowed and are essentially illegible. Querying the cursor position on non-Windows systems (plot twist, Microsoft suggests using VT escape sequences on newer systems) is extremely cumbersome so let's do something different. Before printing anything let's save the cursor position and clear the screen below the cursor, this way we ensure there's absolutely no trace of stale data on screen, and after the message is printed we simply restore it.
1 parent 0447a2c
Changed files (1)
lib
lib/std/Progress.zig
@@ -59,10 +59,6 @@ done: bool = true,
 /// while it was still being accessed by the `refresh` function.
 update_lock: std.Thread.Mutex = .{},
 
-/// Keeps track of how many columns in the terminal have been output, so that
-/// we can move the cursor back later.
-columns_written: usize = undefined,
-
 /// Represents one unit of progress. Each node can have children nodes, or
 /// one can use integers with `update`.
 pub const Node = struct {
@@ -159,7 +155,6 @@ pub fn start(self: *Progress, name: []const u8, estimated_total_items: usize) !*
         .unprotected_estimated_total_items = estimated_total_items,
         .unprotected_completed_items = 0,
     };
-    self.columns_written = 0;
     self.prev_refresh_timestamp = 0;
     self.timer = try std.time.Timer.start();
     self.done = false;
@@ -187,64 +182,56 @@ pub fn refresh(self: *Progress) void {
     return self.refreshWithHeldLock();
 }
 
+// ED -- Clear screen
+const ED = "\x1b[J";
+// DECSC -- Save cursor position
+const DECSC = "\x1b[s";
+// DECRC -- Restore cursor position
+const DECRC = "\x1b[u";
+
 fn refreshWithHeldLock(self: *Progress) void {
     const is_dumb = !self.supports_ansi_escape_codes and !(std.builtin.os.tag == .windows);
     if (is_dumb and self.dont_print_on_dumb) return;
     const file = self.terminal orelse return;
 
-    const prev_columns_written = self.columns_written;
     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
-        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;
-        } else if (std.builtin.os.tag == .windows) winapi: {
-            var info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined;
-            if (windows.kernel32.GetConsoleScreenBufferInfo(file.handle, &info) != windows.TRUE)
-                unreachable;
-
-            var cursor_pos = windows.COORD{
-                .X = info.dwCursorPosition.X - @intCast(windows.SHORT, self.columns_written),
-                .Y = info.dwCursorPosition.Y,
-            };
-
-            if (cursor_pos.X < 0)
-                cursor_pos.X = 0;
-
-            const fill_chars = @intCast(windows.DWORD, info.dwSize.X - cursor_pos.X);
-
-            var written: windows.DWORD = undefined;
-            if (windows.kernel32.FillConsoleOutputAttribute(
-                file.handle,
-                info.wAttributes,
-                fill_chars,
-                cursor_pos,
-                &written,
-            ) != windows.TRUE) {
-                // Stop trying to write to this file.
-                self.terminal = null;
-                break :winapi;
-            }
-            if (windows.kernel32.FillConsoleOutputCharacterA(
-                file.handle,
-                ' ',
-                fill_chars,
-                cursor_pos,
-                &written,
-            ) != windows.TRUE) unreachable;
-
-            if (windows.kernel32.SetConsoleCursorPosition(file.handle, cursor_pos) != windows.TRUE)
-                unreachable;
-        } else {
-            // we are in a "dumb" terminal like in acme or writing to a file
-            self.output_buffer[end] = '\n';
-            end += 1;
+    // Save the cursor position and clear the part of the screen below.
+    // Clearing only the line is not enough as the terminal may wrap the line
+    // when it becomes too long.
+    var saved_cursor_pos: windows.COORD = undefined;
+    if (self.supports_ansi_escape_codes) {
+        const seq_before = DECSC ++ ED;
+        std.mem.copy(u8, self.output_buffer[end..], seq_before);
+        end += seq_before.len;
+    } else if (std.builtin.os.tag == .windows) winapi: {
+        var info: windows.CONSOLE_SCREEN_BUFFER_INFO = undefined;
+        if (windows.kernel32.GetConsoleScreenBufferInfo(file.handle, &info) != windows.TRUE)
+            unreachable;
+
+        saved_cursor_pos = info.dwCursorPosition;
+        const fill_chars = @intCast(windows.DWORD, info.dwSize.X * (info.dwSize.Y - info.dwCursorPosition.Y) - info.dwCursorPosition.X);
+
+        var written: windows.DWORD = undefined;
+        if (windows.kernel32.FillConsoleOutputAttribute(
+            file.handle,
+            info.wAttributes,
+            fill_chars,
+            saved_cursor_pos,
+            &written,
+        ) != windows.TRUE) {
+            // Stop trying to write to this file.
+            self.terminal = null;
+            break :winapi;
+        }
+        if (windows.kernel32.FillConsoleOutputCharacterA(
+            file.handle,
+            ' ',
+            fill_chars,
+            saved_cursor_pos,
+            &written,
+        ) != windows.TRUE) {
+            unreachable;
         }
-
-        self.columns_written = 0;
     }
 
     if (!self.done) {
@@ -279,6 +266,20 @@ fn refreshWithHeldLock(self: *Progress) void {
         }
     }
 
+    // We're done printing the updated message, restore the cursor position.
+    if (self.supports_ansi_escape_codes) {
+        const seq_after = DECRC;
+        std.mem.copy(u8, self.output_buffer[end..], seq_after);
+        end += seq_after.len;
+    } else if (std.builtin.os.tag == .windows) {
+        if (windows.kernel32.SetConsoleCursorPosition(file.handle, saved_cursor_pos) != windows.TRUE) {
+            unreachable;
+        }
+    } else {
+        self.output_buffer[end] = '\n';
+        end += 1;
+    }
+
     _ = file.write(self.output_buffer[0..end]) catch |e| {
         // Stop trying to write to this file once it errors.
         self.terminal = null;
@@ -293,17 +294,14 @@ pub fn log(self: *Progress, comptime format: []const u8, args: anytype) void {
         self.terminal = null;
         return;
     };
-    self.columns_written = 0;
 }
 
 fn bufWrite(self: *Progress, end: *usize, comptime format: []const u8, args: anytype) void {
     if (std.fmt.bufPrint(self.output_buffer[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;
         },
     }
@@ -311,7 +309,6 @@ fn bufWrite(self: *Progress, end: *usize, comptime format: []const u8, args: any
     const max_end = self.output_buffer.len - bytes_needed_for_esc_codes_at_end;
     if (end.* > max_end) {
         const suffix = "... ";
-        self.columns_written = self.columns_written - (end.* - max_end) + suffix.len;
         std.mem.copy(u8, self.output_buffer[max_end..], suffix);
         end.* = max_end + suffix.len;
     }