feat: integrate zwanzig linter #168
Conversation
Add zwanzig v0.1.1 as a Zig dependency and wire up a `zig build lint` step. The Justfile lint target now also invokes the Zig linter after shellcheck, ruff, and zig fmt.
- Add .zwanzig.json to disable identifier-style rule (too many false positives in C interop code) - Fix dupe-import: add colors import at top of UI component files - Fix unused-decl: remove MAX_NON_ROOT_WORKTREES, UiComponent import, TomlPersistenceSerialized, writeTomlString, posix import - Add debug logging for swallowed errors in notify.zig networking code Remaining swallowed-error warnings are intentional fallbacks for non-critical UI display operations.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a4c58395e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (b.lazyDependency("zwanzig", .{ | ||
| .target = target, | ||
| .optimize = optimize, | ||
| })) |zw| { |
There was a problem hiding this comment.
The lint step builds zwanzig using the app’s target settings. When a developer or CI runs zig build lint -Dtarget=... for cross‑compilation, zwanzig will be built for the target and then executed on the host via addRunArtifact, which fails (e.g., Windows binary on Linux). Build tools that are executed during the build should use the host target (e.g., b.host or omit .target so it defaults to host) to keep zig build lint working in cross builds.
Useful? React with 👍 / 👎.
Update zwanzig to v0.2.0 which supports inline suppressions. - Remove global identifier-style disable from .zwanzig.json - Add file-level suppression to c.zig (C interop) - Add line-level suppressions for type aliases and POSIX/ObjC interop - Fix legit naming issues: inConfirm→in_confirm, sizeCopy→size_copy
v0.2.1 fixes type alias detection for simple aliases, type extraction, and function pointer types. Removed 5 suppressions that are no longer needed (Pty, Fd, ReadonlyHandler, ObjcMsgSend*). Remaining suppressions are for issues still present in v0.2.1: - Conditional type aliases (InputSourceTracker) - Variables misidentified as type aliases (status, type_id) - SCREAMING_SNAKE_CASE constants (METRIC_COUNT, TerminalKeyPrefix)
Issue: Update zwanzig linter to v0.2.4, remove unnecessary suppressions,
and fix all legitimate linting issues while keeping suppressions only for
FFI/POSIX names that must match external conventions.
Solution: Updated zwanzig dependency from v0.2.1 to v0.2.4. Fixed ~70+
identifier-style issues by converting SCREAMING_CASE constants to snake_case
across 20+ files. Fixed ~30+ swallowed-error issues by adding proper error
logging using the pattern `catch |err| blk: { log.warn(...); break :blk fallback; }`.
Fixed shadowing issues by renaming local variables (e.g., padding → scaled_padding).
Kept suppressions only for c.zig (FFI names), pty.zig (POSIX names), and one
intentional empty catch in shell.zig (forked child process where logging is
impractical).
There was a problem hiding this comment.
Pull request overview
This PR integrates the zwanzig static analyzer and linter for Zig code, addressing 158 linting issues across the codebase. The integration includes package dependency setup, build system configuration, and systematic code improvements.
Changes:
- Added zwanzig v0.2.4 as a dependency with lazy loading in the build system
- Renamed constants from SCREAMING_SNAKE_CASE to snake_case per Zig conventions
- Added error logging for previously swallowed errors in UI and networking code
- Removed unused code and imports
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| build.zig.zon | Adds zwanzig v0.2.4 dependency |
| build.zig | Configures lint step using zwanzig |
| justfile | Integrates zig build lint into CI pipeline |
| src/ui/components/*.zig | Renames constants to snake_case, adds error logging |
| src/shell.zig | Renames constants, adds error logging |
| src/session/*.zig | Renames constants, adds error logging for networking |
| src/render/renderer.zig | Renames rendering constants |
| src/pty.zig | Adds zwanzig disable comment for C interop |
| src/c.zig | Adds zwanzig disable comment for C bindings |
| src/config.zig | Renames constants, removes unused code |
| src/font*.zig | Renames constants, adds error logging |
| src/app/*.zig | Renames constants throughout application layer |
| src/cwd.zig | Removes unused import |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Rename module constant `fade_width` to `fade_gradient_width` so local scaled variable can use `fade_width` consistently with other scaled values - Rename function parameter back to `fade_width` for caller consistency - Improve error messages in worktree_overlay.zig to be more specific: "failed to create title texture" and "failed to create worktree message texture"
Summary
Integrates the zwanzig static analyzer and linter for Zig code into the project.
Solution
zig build lintstep in build.zig using lazy dependency loadingzig build lintin the existinglinttarget.zwanzig-cache/to.gitignoreAfter integration, reviewed and addressed the 158 issues found:
identifier-style(100).zwanzig.json- 81 were false positives from C interop code inc.zigdupe-import(10)colorsimport at file top instead of inline@import()unused-decl(5)MAX_NON_ROOT_WORKTREES, unused imports, unused functions)swallowed-error(42→38)notify.zig; remaining are intentional UI fallbacksFinal state: 158 → 38 issues (76% reduction). Remaining warnings are intentional graceful degradation patterns in UI code.
Usage