feat(zigcli): add progress library and rename binary#78
feat(zigcli): add progress library and rename binary#78jiacai2050 wants to merge 4 commits intomainfrom
Conversation
Adds the `zigcli.progress` package to provide reusable progress bars and spinners. Renames the standalone binary from `progress` to `progress-it` to resolve naming collisions. Includes a demonstration program and documentation for the new library.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a well-designed progress package for creating progress bars and spinners, which is a great addition to the zigcli toolkit. The code is robust, with careful attention to memory management and a clean API. The renaming of the progress binary to progress-it to avoid conflicts is also a sensible change. I've included a few minor suggestions to improve documentation consistency and enhance code maintainability.
There was a problem hiding this comment.
Pull request overview
Adds a reusable progress rendering library to the zigcli root module and updates the build to ship a renamed standalone binary to avoid name collisions.
Changes:
- Introduces
src/progress.zigproviding progress bars, spinners, multi-progress rendering, plus reader/writer wrappers and tests. - Exports the new package from
src/lib.zigaszigcli.progressand adds anexamples/progress-demo.zigdemo program. - Renames the standalone binary from
progresstoprogress-itinbuild.zigand updates the corresponding compile-time error message.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/progress.zig | New reusable progress library (bar/spinner/multi) with tests. |
| src/lib.zig | Re-exports progress from the zigcli root module. |
| src/bin/progress-it.zig | Updates platform compile error string to match renamed binary. |
| examples/progress-demo.zig | Adds a demo program showing library usage. |
| build.zig | Registers the progress-demo example and renames progress binary to progress-it with updated platform gating. |
| .github/copilot-instructions.md | Adds repo guidance (needs small updates to match new export and binary rename). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const rate_text = try formatCount( | ||
| arena, | ||
| self.style.unit, | ||
| @intFromFloat(rate), |
There was a problem hiding this comment.
@intFromFloat(rate) can trap if the computed rate exceeds the integer range (e.g., very large position with minimal elapsed time). Consider clamping the rate before converting, or formatting the rate as a float/string without converting to an integer.
| const rate_text = try formatCount( | |
| arena, | |
| self.style.unit, | |
| @intFromFloat(rate), | |
| const max_rate_f64 = @as(f64, @floatFromInt(std.math.maxInt(u64))); | |
| const clamped_rate = std.math.clamp(rate, 0.0, max_rate_f64); | |
| const rate_int = @as(u64, @intFromFloat(clamped_rate)); | |
| const rate_text = try formatCount( | |
| arena, | |
| self.style.unit, | |
| rate_int, |
| const rate_text = try formatCount( | ||
| arena, | ||
| self.style.unit, | ||
| @intFromFloat(rate), |
There was a problem hiding this comment.
Same issue as the bar stats: @intFromFloat(rate) may trap if the computed rate is outside the integer range. Clamp before converting or avoid the integer conversion when formatting the rate.
| const rate_text = try formatCount( | |
| arena, | |
| self.style.unit, | |
| @intFromFloat(rate), | |
| const max_rate_int: u64 = std.math.maxInt(u64); | |
| const max_rate_float = @as(f64, @floatFromInt(max_rate_int)); | |
| const clamped_rate = if (rate > max_rate_float) max_rate_float else rate; | |
| const rate_text = try formatCount( | |
| arena, | |
| self.style.unit, | |
| @intFromFloat(clamped_rate), |
| if (visible_index < visible_count) { | ||
| const progress_ptr = self.nthVisible(visible_index); | ||
| try progress_ptr.writeSnapshotAt( | ||
| writer, | ||
| now_ms, | ||
| self.draw_target.width_columns, |
There was a problem hiding this comment.
renderToWriterAt calls nthVisible() inside the render loop; nthVisible() linearly scans items each time, making rendering O(n^2) in the number of progress items. Consider iterating items once (skipping cleared entries) or building a temporary list of visible pointers for the current render to keep rendering O(n).
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replace nested if-else blocks with a switch statement to improve code readability. Group spinner-specific validation logic into its corresponding case arm.
Adds the
zigcli.progresspackage to provide reusable progress bars and spinners. Renames the standalone binary fromprogresstoprogress-itto resolve naming collisions. Includes a demonstration program and documentation for the new library.