refactor(bin): rename binary to zfetch and modularize#72
Conversation
Refactor monolithic implementation into a modular architecture with OS-specific modules (Linux, macOS, FreeBSD). Add JSON output support. Rename binary to zfetch to reflect its Zig origin.
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 refactors the system information fetching tool by modularizing its codebase and expanding its platform compatibility. The changes aim to improve maintainability, facilitate future development, and provide more flexible output options for users. The core functionality remains the same, but the underlying structure is now more robust and extensible. 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 is a significant and well-executed refactoring of the fastfetch binary into the newly named zfetch. The modular architecture with OS-specific implementations for Linux, macOS, and now FreeBSD is a major improvement for maintainability and extensibility. The addition of JSON output is also a valuable feature. While the overall changes are excellent, I've identified a few issues, including a critical bug in package counting on FreeBSD, a high-severity bug in package counting on macOS, and a potential crash in disk usage calculation. I've also provided suggestions to improve performance and code clarity.
src/bin/zfetch/common.zig
Outdated
| var dir = fs.openDirAbsolute( | ||
| "/var/db/pkg/local.sqlite", | ||
| .{}, | ||
| ) catch { | ||
| // Fallback: count directories in /var/db/pkg. | ||
| var pkg_dir = fs.openDirAbsolute( | ||
| "/var/db/pkg", | ||
| .{ .iterate = true }, | ||
| ) catch return "Unknown"; | ||
| defer pkg_dir.close(); | ||
| var count: u32 = 0; | ||
| var iter = pkg_dir.iterate(); | ||
| while (try iter.next()) |entry| { | ||
| if (entry.kind == .directory) count += 1; | ||
| } | ||
| if (count > 0) { | ||
| return fmt.allocPrint( | ||
| allocator, | ||
| "{d} (pkg)", | ||
| .{count}, | ||
| ); | ||
| } | ||
| return "Unknown"; | ||
| }; | ||
| defer dir.close(); | ||
| return "Unknown"; |
There was a problem hiding this comment.
The implementation of getPackagesPkg for FreeBSD appears to be incorrect. It attempts to use fs.openDirAbsolute on a file path (/var/db/pkg/local.sqlite), which will fail. If it were to succeed (e.g., if the path was a directory), the function would incorrectly return "Unknown" without processing it. The logic for handling the SQLite database is missing.
This should be refactored to either correctly implement the SQLite database query or be simplified to only contain the fallback logic if the primary implementation is not ready.
// TODO: Implement sqlite query for /var/db/pkg/local.sqlite
// For now, using fallback to count directories in /var/db/pkg.
var pkg_dir = fs.openDirAbsolute(
"/var/db/pkg",
.{ .iterate = true },
) catch return "Unknown";
defer pkg_dir.close();
var count: u32 = 0;
var iter = pkg_dir.iterate();
while (try iter.next()) |entry| {
if (entry.kind == .directory) count += 1;
}
if (count > 0) {
return fmt.allocPrint(
allocator,
"{d} (pkg)",
.{count},
);
}
return "Unknown";
src/bin/zfetch/common.zig
Outdated
| } | ||
| } | ||
| if (dup) continue; | ||
| assert(seen_count < seen_dev.len); |
There was a problem hiding this comment.
Using assert here could cause the program to crash in debug builds if more than 8 mount points with unique device IDs are processed. It's safer to handle this case gracefully with a runtime check, for example by skipping devices that would overflow the seen_dev array.
if (seen_count >= seen_dev.len) continue;
| .{count}, | ||
| ); | ||
| } | ||
| break; // Only use the first found path. |
There was a problem hiding this comment.
The break statement here is causing a bug in package counting. If Homebrew is installed at /opt/homebrew/Cellar but contains no packages, the loop will break and will not check the alternative path /usr/local/Cellar. This will result in an incorrect package count of zero. The break should be removed to ensure all potential Homebrew installation paths are checked.
build.zig
Outdated
| if (target_os != .macos and target_os != .linux and target_os != .freebsd) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
This condition check for supported operating systems can be made more concise and readable by using a switch statement. This is generally preferred in Zig for matching against multiple enum values.
switch (target_os) {
.macos, .linux, .freebsd => {},
else => return null,
}
src/bin/zfetch.zig
Outdated
| const formatted = try fmt.allocPrint( | ||
| allocator, | ||
| "{f}", | ||
| .{std.json.fmt(info, .{})}, | ||
| ); | ||
| try writer.writeAll(formatted); |
There was a problem hiding this comment.
The current implementation for JSON output allocates a string for the entire JSON object before writing it to stdout. This can be inefficient, especially if the system information becomes more detailed in the future. It's better to stream the JSON output directly to the writer to avoid the intermediate allocation.
try std.json.stringify(info, .{}, writer);
There was a problem hiding this comment.
Pull request overview
Refactors the former fastfetch binary into a new zfetch binary with a shared common module and per-OS backends, and introduces a --format=json output mode.
Changes:
- Renames the binary from
fastfetchtozfetchand updates the build to compile/link it on macOS/Linux/FreeBSD only. - Splits platform-specific logic into
src/bin/zfetch/{linux,macos,freebsd}.zigwith shared helpers insrc/bin/zfetch/common.zig. - Updates documentation to reflect the renamed program and new sample output.
Reviewed changes
Copilot reviewed 2 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/bin/zfetch.zig | New entrypoint: argument parsing, info collection, text/JSON output. |
| src/bin/zfetch/common.zig | Shared helpers (disk, IP, uptime/memory parsing, package counting) + tests moved here. |
| src/bin/zfetch/linux.zig | Linux backend wired to common /proc//sys helpers. |
| src/bin/zfetch/macos.zig | macOS backend using sysctl, CoreGraphics, CoreFoundation, IOKit. |
| src/bin/zfetch/freebsd.zig | FreeBSD backend using sysctl + shared helpers. |
| src/bin/fastfetch.zig | Removed monolithic implementation. |
| docs/content/programs/zfetch.org | Renames docs page and updates demo snippet/output. |
| build.zig | Swaps fastfetch for zfetch and adjusts platform gating/linking. |
Comments suppressed due to low confidence (1)
docs/content/programs/zfetch.org:10
- The feature list says zfetch "Supports both macOS and Linux", but the implementation/build rules also include FreeBSD. Update the documentation to reflect FreeBSD support (or clarify the actual supported set).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Add FreeBSD targets to CI and release scripts. Refactor zfetch to use a common POSIX module for host info collection and update build logic for platform-specific exclusions.
Fix project name mismatch in build blacklist ("zig-fetch").
Replace strict assertions with defensive continues to prevent edge-case failures.
Remove unused imports and refactor JSON formatting for improved clarity.
Refactor monolithic implementation into a modular architecture with OS-specific modules (Linux, macOS, FreeBSD). Add JSON output support. Rename binary to zfetch to reflect its Zig origin.