feat: add runtime table builder and pretty-csv utility#73
Conversation
Implements `TableBuilder` for dynamic row management during runtime. Introduces `pretty-csv` binary for rendering text tables from CSV/TSV data. Updates install script and documentation coverage.
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 introduces a powerful new command-line tool for pretty-printing CSV data, significantly upgrades 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new pretty-csv utility for rendering CSV/TSV data as tables, and adds a TableBuilder to the pretty-table module for dynamically building tables at runtime. The documentation has also been updated to cover these new features.
The implementation of TableBuilder is well-designed and includes relevant tests.
However, the new pretty-csv binary has some significant issues. It re-implements a large part of the table rendering logic instead of using the pretty-table module, leading to code duplication. Additionally, its CSV parsing logic has several correctness bugs related to RFC 4180 compliance, such as incorrect handling of escaped quotes and no support for multiline fields. There is also a minor issue in the documentation with a hardcoded user path in an example.
My review includes suggestions to address these points by refactoring pretty-csv and fixing the parsing logic.
| fn parseLine( | ||
| allocator: mem.Allocator, | ||
| line: []const u8, | ||
| delim: u8, | ||
| ) ![]const []const u8 { |
There was a problem hiding this comment.
The CSV parsing logic in this function has several issues regarding RFC 4180 compliance, which can lead to incorrect parsing of valid CSV data:
- No multiline field support: The input is split by
\nupfront (inmain), which prevents the parser from handling newlines inside quoted fields. - Incorrect field counting: The initial loop to count fields does not correctly handle escaped quotes (
""). For a line like"a,""b",c", it will miscalculate the number of fields, leading to parsing errors. - No un-escaping of quotes: The parser slices field content directly from the input buffer. It does not process the content to un-escape
""into a single".
Consider implementing a more robust state machine for CSV parsing that correctly handles these cases according to RFC 4180.
There was a problem hiding this comment.
Pull request overview
Adds a runtime-oriented table construction API to pretty-table and introduces a new pretty-csv CLI that renders CSV/TSV into aligned terminal tables, with accompanying install-script and documentation updates.
Changes:
- Expose
pretty-table.Separator.Position/get()publicly and addTableBuilder(N)for dynamic row accumulation. - Add
pretty-csvbinary to parse CSV/TSV input and render it as a bordered table (including transpose, width limiting, and column filtering). - Update install script and docs to include
pretty-csv, expandpretty-table/simargsdocs, and add docs forgitignore.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mod/pretty-table.zig | Makes separator positioning API public and adds TableBuilder plus tests. |
| src/bin/pretty-csv.zig | New CLI: parses input, computes widths, and renders tables (uses pretty-table separators). |
| docs/static/install.sh | Adds pretty-csv to default installed binaries. |
| docs/content/programs/pretty-csv.org | New user documentation for pretty-csv. |
| docs/content/packages/simargs.org | Updates/expands simargs documentation and examples. |
| docs/content/packages/pretty-table.org | Expands pretty-table documentation and adds TableBuilder section. |
| docs/content/packages/gitignore.org | Adds documentation page for the gitignore package. |
| docs/content/install.org | Updates install docs to mention/import three packages. |
| build.zig | Registers the pretty-csv binary in the build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
docs/content/programs/pretty-csv.org
Outdated
| - Three border styles: =ascii=, =box=, =dos=. | ||
| - Transpose mode: show each record as a vertical key-value block. | ||
| - Column selection: display only specific columns. | ||
| - Supports quoted fields (RFC 4180). |
There was a problem hiding this comment.
The docs claim "Supports quoted fields (RFC 4180)", but the current implementation splits on newlines before parsing and doesn't unescape doubled quotes inside quoted fields. Either narrow the claim (e.g., single-line quoted fields only) or expand the parser to match RFC 4180 behavior.
| - Supports quoted fields (RFC 4180). | |
| - Supports basic quoted fields (single-line; limited RFC 4180 support). |
| * Features | ||
| - Supported data type: | ||
| - All [[https://ziglang.org/documentation/master/#Primitive-Types][primitive types]], such as =i8=, =f32=, =bool= | ||
| - All [[https://ziglang.org/documentation/0.15.2/#Primitive-Types][primitive types]], such as =i8=, =f32=, =bool= |
There was a problem hiding this comment.
This doc links to Zig 0.15.2 docs, but the repo states it targets Zig 0.15.1. To avoid confusing users (and to keep references consistent with the supported toolchain), consider linking to 0.15.1 (or to the "master" docs if you intentionally want the rolling reference).
| - All [[https://ziglang.org/documentation/0.15.2/#Primitive-Types][primitive types]], such as =i8=, =f32=, =bool= | |
| - All [[https://ziglang.org/documentation/0.15.1/#Primitive-Types][primitive types]], such as =i8=, =f32=, =bool= |
src/mod/pretty-table.zig
Outdated
| }; | ||
|
|
||
| const Position = enum { First, Text, Sep, Last }; | ||
| pub const Position = enum { First, Text, Sep, Last }; |
There was a problem hiding this comment.
Separator.Position is now part of the public API, but its tags are First/Text/Sep/Last (PascalCase), while other public enums in this module use lowercase tags (Mode, Align, Color). Consider renaming these tags to lowercase before stabilizing the API to keep enum naming consistent for callers.
| pub const Position = enum { First, Text, Sep, Last }; | |
| pub const Position = enum { first, text, sep, last }; |
Refactor pretty-csv to use the new DynTable module instead of custom rendering logic. Remove duplicate width detection and truncation implementation to leverage library features. Update documentation to reflect changes in pretty-table and add new packages.
Rename API types for semantic clarity:
- Rename `TableBuilder(N)` to `Table(N).Owned`.
- Rename `DynTable` to `RuntimeTable`.
Add support for more complex data representations:
- Add `transpose` mode and row separators to `Table(N)`.
- Add footer support, per-column alignment, and `"{f}"` formatting to `RuntimeTable`.
- Expose `--row-separator` and `--right-columns` flags to CLI tools.
This enables flexible row representations and precise numerical alignment in reports.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the pretty-csv utility and significantly enhances the pretty-table module. The pretty-table module now supports runtime table building with Table(N).Owned and RuntimeTable, offering dynamic column counts, transposition, and cell truncation. The pretty-csv binary effectively leverages these new features to provide a robust tool for rendering CSV/TSV data as aligned tables. The documentation has been thoroughly updated to reflect these changes, including new examples and API descriptions. Overall, this is a substantial and well-documented feature addition.
However, there are a few areas for improvement regarding robustness and cross-platform compatibility:
- The fixed-size scratch buffer for column filtering in
pretty-csvcould lead to silent data loss if the number of columns exceeds its capacity. - The fixed-size output buffer for
pretty-csvcould cause the program to crash on very large outputs. - The text truncation logic in
pretty-tableis not UTF-8 aware, which might lead to malformed output or incorrect visual widths for multi-byte characters. - The terminal width detection in
pretty-tableis POSIX-specific and lacks a Windows implementation, limiting its cross-platform utility for that specific feature.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove hardcoded 256 column limit in filtering by utilizing dynamic memory allocation. Previously, columns exceeding this threshold were silently truncated. Improve visual truncation accuracy by counting UTF-8 code points instead of bytes for accurate alignment and representation of multi-byte characters.
Implements
TableBuilderfor dynamic row management during runtime. Introducespretty-csvbinary for rendering text tables from CSV/TSV data. Updates install script and documentation coverage.