Commit 2d5b2bf1c9

Andrew Kelley <andrew@ziglang.org>
2019-10-18 03:46:41
improve progress reporting
* use erase rest of line escape code. * use `stderr.supportsAnsiEscapeCodes` rather than `isTty`. * respect `--color off` * avoid unnecessary recursion * add `Progress.log` * disable the progress std lib test since it's noisy and uses `time.sleep()`. * enable/integrate progress printing with the default test runner
1 parent 2999910
Changed files (8)
lib/std/special/test_runner.zig
@@ -2,28 +2,33 @@ const std = @import("std");
 const io = std.io;
 const builtin = @import("builtin");
 const test_fn_list = builtin.test_functions;
-const warn = std.debug.warn;
 
-pub fn main() !void {
+pub fn main() anyerror!void {
     var ok_count: usize = 0;
     var skip_count: usize = 0;
-    for (test_fn_list) |test_fn, i| {
-        warn("{}/{} {}...", i + 1, test_fn_list.len, test_fn.name);
+    var progress = std.Progress{};
+    const root_node = progress.start("Test", test_fn_list.len) catch |err| switch (err) {
+        // TODO still run tests in this case
+        error.TimerUnsupported => @panic("timer unsupported"),
+    };
 
+    for (test_fn_list) |test_fn, i| {
+        var test_node = root_node.start(test_fn.name, null);
+        test_node.activate();
         if (test_fn.func()) |_| {
             ok_count += 1;
-            warn("OK\n");
+            test_node.end();
         } else |err| switch (err) {
             error.SkipZigTest => {
                 skip_count += 1;
-                warn("SKIP\n");
+                test_node.end();
+                progress.log("{}...SKIP\n", test_fn.name);
             },
             else => return err,
         }
     }
-    if (ok_count == test_fn_list.len) {
-        warn("All tests passed.\n");
-    } else {
-        warn("{} passed; {} skipped.\n", ok_count, skip_count);
+    root_node.end();
+    if (ok_count != test_fn_list.len) {
+        progress.log("{} passed; {} skipped.\n", ok_count, skip_count);
     }
 }
lib/std/progress.zig
@@ -26,10 +26,6 @@ pub const Progress = struct {
     /// with each refresh.
     output_buffer: [100]u8 = undefined,
 
-    /// 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,
-
     /// How many nanoseconds between writing updates to the terminal.
     refresh_rate_ns: u64 = 50 * std.time.millisecond,
 
@@ -38,6 +34,10 @@ pub const Progress = struct {
 
     done: bool = true,
 
+    /// 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 {
@@ -99,8 +99,7 @@ pub const Progress = struct {
     /// API to return Progress rather than accept it as a parameter.
     pub fn start(self: *Progress, name: []const u8, estimated_total_items: ?usize) !*Node {
         if (std.io.getStdErr()) |stderr| {
-            const is_term = stderr.isTty();
-            self.terminal = if (is_term) stderr else null;
+            self.terminal = if (stderr.supportsAnsiEscapeCodes()) stderr else null;
         } else |_| {
             self.terminal = null;
         }
@@ -111,8 +110,8 @@ pub const Progress = struct {
             .name = name,
             .estimated_total_items = estimated_total_items,
         };
-        self.prev_refresh_timestamp = 0;
         self.columns_written = 0;
+        self.prev_refresh_timestamp = 0;
         self.timer = try std.time.Timer.start();
         self.done = false;
         return &self.root;
@@ -133,20 +132,42 @@ pub const Progress = struct {
         const prev_columns_written = self.columns_written;
         var end: usize = 0;
         if (self.columns_written > 0) {
+            // restore cursor position
             end += (std.fmt.bufPrint(self.output_buffer[end..], "\x1b[{}D", self.columns_written) catch unreachable).len;
             self.columns_written = 0;
-        }
 
-        if (!self.done) {
-            self.bufWriteNode(self.root, &end);
-            self.bufWrite(&end, "...");
+            // clear rest of line
+            end += (std.fmt.bufPrint(self.output_buffer[end..], "\x1b[0K") catch unreachable).len;
         }
 
-        if (prev_columns_written > self.columns_written) {
-            const amt = prev_columns_written - self.columns_written;
-            std.mem.set(u8, self.output_buffer[end .. end + amt], ' ');
-            end += amt;
-            end += (std.fmt.bufPrint(self.output_buffer[end..], "\x1b[{}D", amt) catch unreachable).len;
+        if (!self.done) {
+            var need_ellipse = false;
+            var maybe_node: ?*Node = &self.root;
+            while (maybe_node) |node| {
+                if (need_ellipse) {
+                    self.bufWrite(&end, "...");
+                }
+                need_ellipse = false;
+                if (node.name.len != 0 or node.estimated_total_items != null) {
+                    if (node.name.len != 0) {
+                        self.bufWrite(&end, "{}", node.name);
+                        need_ellipse = true;
+                    }
+                    if (node.estimated_total_items) |total| {
+                        if (need_ellipse) self.bufWrite(&end, " ");
+                        self.bufWrite(&end, "[{}/{}] ", node.completed_items, total);
+                        need_ellipse = false;
+                    } else if (node.completed_items != 0) {
+                        if (need_ellipse) self.bufWrite(&end, " ");
+                        self.bufWrite(&end, "[{}] ", node.completed_items);
+                        need_ellipse = false;
+                    }
+                }
+                maybe_node = node.recently_updated_child;
+            }
+            if (need_ellipse) {
+                self.bufWrite(&end, "...");
+            }
         }
 
         _ = file.write(self.output_buffer[0..end]) catch |e| {
@@ -156,25 +177,14 @@ pub const Progress = struct {
         self.prev_refresh_timestamp = self.timer.read();
     }
 
-    fn bufWriteNode(self: *Progress, node: Node, end: *usize) void {
-        if (node.name.len != 0 or node.estimated_total_items != null) {
-            if (node.name.len != 0) {
-                self.bufWrite(end, "{}", node.name);
-                if (node.recently_updated_child != null or node.estimated_total_items != null or
-                    node.completed_items != 0)
-                {
-                    self.bufWrite(end, "...");
-                }
-            }
-            if (node.estimated_total_items) |total| {
-                self.bufWrite(end, "[{}/{}] ", node.completed_items, total);
-            } else if (node.completed_items != 0) {
-                self.bufWrite(end, "[{}] ", node.completed_items);
-            }
-        }
-        if (node.recently_updated_child) |child| {
-            self.bufWriteNode(child.*, end);
-        }
+    pub fn log(self: *Progress, comptime format: []const u8, args: ...) void {
+        const file = self.terminal orelse return;
+        self.refresh();
+        file.outStream().stream.print(format, args) catch {
+            self.terminal = null;
+            return;
+        };
+        self.columns_written = 0;
     }
 
     fn bufWrite(self: *Progress, end: *usize, comptime format: []const u8, args: ...) void {
@@ -200,6 +210,12 @@ pub const Progress = struct {
 };
 
 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.
+        return error.SkipZigTest;
+    }
     var progress = Progress{};
     const root_node = try progress.start("", 100);
     defer root_node.end();
@@ -235,7 +251,7 @@ test "basic functionality" {
         var node = root_node.start("this is a really long name designed to activate the truncation code. let's find out if it works", null);
         node.activate();
         std.time.sleep(10 * std.time.millisecond);
-        progress.maybeRefresh();
+        progress.refresh();
         std.time.sleep(10 * std.time.millisecond);
         node.end();
     }
src/codegen.cpp
@@ -10479,11 +10479,11 @@ ZigPackage *codegen_create_package(CodeGen *g, const char *root_src_dir, const c
 }
 
 CodeGen *create_child_codegen(CodeGen *parent_gen, Buf *root_src_path, OutType out_type,
-        ZigLibCInstallation *libc, const char *name, Stage2ProgressNode *child_progress_node)
+        ZigLibCInstallation *libc, const char *name, Stage2ProgressNode *parent_progress_node)
 {
-    if (!child_progress_node) {
-        child_progress_node = stage2_progress_start(parent_gen->progress_node, name, strlen(name), 0);
-    }
+    Stage2ProgressNode *child_progress_node = stage2_progress_start(
+            parent_progress_node ? parent_progress_node : parent_gen->progress_node,
+            name, strlen(name), 0);
 
     CodeGen *child_gen = codegen_create(nullptr, root_src_path, parent_gen->zig_target, out_type,
         parent_gen->build_mode, parent_gen->zig_lib_dir, libc, get_stage1_cache_path(), false, child_progress_node);
src/main.cpp
@@ -506,8 +506,6 @@ int main(int argc, char **argv) {
     ZigList<const char *> llvm_argv = {0};
     llvm_argv.append("zig (LLVM option parsing)");
 
-    Stage2ProgressNode *root_progress_node = stage2_progress_start_root(stage2_progress_create(), "", 0, 0);
-
     if (argc >= 2 && strcmp(argv[1], "build") == 0) {
         Buf zig_exe_path_buf = BUF_INIT;
         if ((err = os_self_exe_path(&zig_exe_path_buf))) {
@@ -589,6 +587,7 @@ int main(int argc, char **argv) {
             Buf *cache_dir_buf = buf_create_from_str(cache_dir);
             full_cache_dir = os_path_resolve(&cache_dir_buf, 1);
         }
+        Stage2ProgressNode *root_progress_node = stage2_progress_start_root(stage2_progress_create(), "", 0, 0);
 
         CodeGen *g = codegen_create(main_pkg_path, build_runner_path, &target, OutTypeExe,
                 BuildModeDebug, override_lib_dir, nullptr, &full_cache_dir, false, root_progress_node);
@@ -965,6 +964,10 @@ int main(int argc, char **argv) {
         return EXIT_FAILURE;
     }
 
+    Stage2Progress *progress = stage2_progress_create();
+    Stage2ProgressNode *root_progress_node = stage2_progress_start_root(progress, "", 0, 0);
+    if (color == ErrColorOff) stage2_progress_disable_tty(progress);
+
     init_all_targets();
 
     ZigTarget target;
src/userland.cpp
@@ -87,3 +87,4 @@ Stage2ProgressNode *stage2_progress_start(Stage2ProgressNode *node,
 }
 void stage2_progress_end(Stage2ProgressNode *node) {}
 void stage2_progress_complete_one(Stage2ProgressNode *node) {}
+void stage2_progress_disable_tty(Stage2Progress *progress) {}
src/userland.h
@@ -163,6 +163,8 @@ struct Stage2ProgressNode;
 // ABI warning
 ZIG_EXTERN_C Stage2Progress *stage2_progress_create(void);
 // ABI warning
+ZIG_EXTERN_C void stage2_progress_disable_tty(Stage2Progress *progress);
+// ABI warning
 ZIG_EXTERN_C void stage2_progress_destroy(Stage2Progress *progress);
 // ABI warning
 ZIG_EXTERN_C Stage2ProgressNode *stage2_progress_start_root(Stage2Progress *progress,
src-self-hosted/stage1.zig
@@ -470,13 +470,23 @@ export fn stage2_progress_destroy(progress: *std.Progress) void {
 }
 
 // ABI warning
-export fn stage2_progress_start_root(progress: *std.Progress, name_ptr: [*]const u8, name_len: usize, estimated_total_items: usize) *std.Progress.Node {
+export fn stage2_progress_start_root(
+    progress: *std.Progress,
+    name_ptr: [*]const u8,
+    name_len: usize,
+    estimated_total_items: usize,
+) *std.Progress.Node {
     return progress.start(
         name_ptr[0..name_len],
         if (estimated_total_items == 0) null else estimated_total_items,
     ) catch @panic("timer unsupported");
 }
 
+// ABI warning
+export fn stage2_progress_disable_tty(progress: *std.Progress) void {
+    progress.terminal = null;
+}
+
 // ABI warning
 export fn stage2_progress_start(
     node: *std.Progress.Node,
test/cli.zig
@@ -87,7 +87,7 @@ fn exec(cwd: []const u8, argv: []const []const u8) !ChildProcess.ExecResult {
 fn testZigInitLib(zig_exe: []const u8, dir_path: []const u8) !void {
     _ = try exec(dir_path, [_][]const u8{ zig_exe, "init-lib" });
     const test_result = try exec(dir_path, [_][]const u8{ zig_exe, "build", "test" });
-    testing.expect(std.mem.endsWith(u8, test_result.stderr, "All tests passed.\n"));
+    testing.expect(std.mem.eql(u8, test_result.stderr, ""));
 }
 
 fn testZigInitExe(zig_exe: []const u8, dir_path: []const u8) !void {
@@ -136,6 +136,6 @@ fn testMissingOutputPath(zig_exe: []const u8, dir_path: []const u8) !void {
     const output_path = try fs.path.join(a, [_][]const u8{ "does", "not", "exist" });
     const source_path = try fs.path.join(a, [_][]const u8{ "src", "main.zig" });
     _ = try exec(dir_path, [_][]const u8{
-        zig_exe, "build-exe", source_path, "--output-dir", output_path
+        zig_exe, "build-exe", source_path, "--output-dir", output_path,
     });
 }