Repository navigation
Pre Commit Hook - #9
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesThe CLI now accepts multiple files or directories, processes targets independently, and reports target errors after collection. Tests cover ordered multi-target parsing. Documentation and the pre-commit manifest describe multi-target usage and Zig import-ordering hooks. Multi-target CLI and integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant parseArgs
participant runMain
participant Filesystem
CLI->>parseArgs: collect positional targets
parseArgs-->>CLI: return Args.targets
CLI->>runMain: process Args.targets
runMain->>Filesystem: inspect each target
Filesystem-->>runMain: target type or access error
runMain-->>CLI: report files, failures, or no-file result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 121: Update the README zsort example to use non-overlapping targets,
replacing the redundant src/main.zig target with a Zig file outside src/, such
as build.zig, so the same file is not processed twice.
- Around line 140-142: Publish the v0.5.0 release tag containing the pre-commit
hooks before users apply this README configuration, and keep the rev set to
v0.5.0 rather than changing it to v0.4.0.
In `@src/zsort.zig`:
- Around line 823-824: Update the target-processing loop around compat.openDir
to catch directory-opening errors instead of propagating them through runMain.
Report the affected target, increment error_count, and continue with subsequent
targets; retain the existing defer cleanup and processing flow for successfully
opened directories.
- Around line 825-826: Release the `.gitignore` pattern allocations after each
directory scan by freeing the slice and its individual patterns once `walkDir`
returns. Update the flow around `loadGitignore` and `walkDir` so cleanup occurs
on both successful and failing scans, while preserving the existing directory
traversal behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1b538ea6-e50a-4d17-9c89-c0fafe1a43d2
📒 Files selected for processing (5)
.pre-commit-hooks.yamlCHANGELOG.mdREADME.mdsrc/zsort.zigsrc/zsort_test.zig
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/zsort.zig (2)
40-51: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the contradictory
escapeTermcontract.Line 43 says allocation failure returns the raw input. Line 44 describes ESC-only escaping. Lines 45-50 define five-byte escaping and
error.OutOfMemorypropagation. Keep one current contract so callers do not rely on removed behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/zsort.zig` around lines 40 - 51, Update the documentation for escapeTerm to remove the obsolete ESC-only behavior and raw-input-on-OOM contract. Keep only the current five-byte terminal-control escaping behavior and the error.OutOfMemory propagation guarantee, ensuring callers rely on this single consistent contract.
817-824: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRelease escaped target strings after error reporting.
When a target contains an escaped control byte,
escapeTermreturns a new allocation. The calls at Lines 817, 824, 838, and 846 pass that allocation directly tocompat.printStderrand lose ownership. Multi-target error handling can retain one allocation per message until process exit.Bind each result to a local value and conditionally free it when its pointer differs from the original target, as done at Lines 887-888.
Also applies to: 838-846
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/zsort.zig` around lines 817 - 824, Update the error-reporting paths around the directory/stat handling and the corresponding blocks at lines 838-846 to store each escapeTerm result in a local variable, pass it to compat.printStderr, then free it when its pointer differs from the original target, matching the existing ownership pattern at lines 887-888.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/zsort.zig`:
- Line 834: Update the recursive scan call in runMain to catch walkDir errors
per target instead of propagating every error with try. Report the affected
target and increment error_count for recoverable filesystem errors, continue
scanning subsequent targets, and preserve propagation of error.OutOfMemory.
---
Outside diff comments:
In `@src/zsort.zig`:
- Around line 40-51: Update the documentation for escapeTerm to remove the
obsolete ESC-only behavior and raw-input-on-OOM contract. Keep only the current
five-byte terminal-control escaping behavior and the error.OutOfMemory
propagation guarantee, ensuring callers rely on this single consistent contract.
- Around line 817-824: Update the error-reporting paths around the
directory/stat handling and the corresponding blocks at lines 838-846 to store
each escapeTerm result in a local variable, pass it to compat.printStderr, then
free it when its pointer differs from the original target, matching the existing
ownership pattern at lines 887-888.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: be2dc3f4-2a7c-44ce-97e0-9f10cf9e3993
📒 Files selected for processing (2)
README.mdsrc/zsort.zig
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
closing #8
Summary by CodeRabbit
New Features
Bug Fixes
Documentation