Repository navigation
Polish shell output - #7
Conversation
📝 WalkthroughWalkthroughChangesThe 0.4.0 release adds conditional ANSI output, stderr diagnostics, revised CLI messages, expanded tests, updated version metadata, and POSIX/WSL2 documentation. CLI output and release
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant runMain
participant zsort
participant compat
participant stdout
participant stderr
runMain->>compat: detect stdout and stderr color support
runMain->>zsort: process target
zsort-->>runMain: return result or diagnostic
runMain->>stdout: write diff, summary, or fix result
runMain->>stderr: write parse or processing error
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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`:
- Around line 264-269: Escape all dynamic text before sending it to terminal
output, preserving raw sequences only for compat.ansi_* constants. In
src/zsort.zig lines 264-269, escape file_path in diff headers; lines 47-52,
prefix in banned-import errors; lines 555-575, arg values in diagnostics; lines
704-712, err_msg in parse errors; lines 751-768, parsed.target in access and
file-status diagnostics; and lines 811-843, file_path and other dynamic
diagnostic content in processing, banned-import, write, and fix messages.
🪄 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: f556ee9f-786f-4373-a634-ca7f168c2163
📒 Files selected for processing (8)
CHANGELOG.mdCONTRIBUTING.mdREADME.mdbuild.zig.zonsrc/compat.zigsrc/version.zigsrc/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 (1)
src/zsort.zig (1)
3-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winEscape terminal control bytes in diff content.
formatUnifiedDiffwrites source lines directly. ESC, CR, BEL, or backspace bytes can alter terminal output. Extend escaping to diff lines and avoid returning raw input whenescapeTermallocation fails. Add regression tests.🤖 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 3 - 7, Update formatUnifiedDiff to escape terminal-control bytes, including ESC, CR, BEL, and backspace, in every emitted diff line before writing it. Ensure escapeTerm does not return raw input when allocation fails; use the established safe fallback behavior instead. Add regression tests covering these bytes in diff content and allocation failure handling.Source: Coding guidelines
🤖 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`:
- Around line 45-59: Update src/zsort.zig lines 45-59 in escapeTerm to encode
ESC, CR, LF, BEL, and backspace, and ensure allocation failure never returns
unsafe original input. Apply escapeTerm to context, deletion, and addition lines
in src/zsort.zig lines 292-295. Add tests in src/zsort_test.zig lines 584-589
covering control bytes in diff body output and the safe allocation-failure
behavior.
---
Outside diff comments:
In `@src/zsort.zig`:
- Around line 3-7: Update formatUnifiedDiff to escape terminal-control bytes,
including ESC, CR, BEL, and backspace, in every emitted diff line before writing
it. Ensure escapeTerm does not return raw input when allocation fails; use the
established safe fallback behavior instead. Add regression tests covering these
bytes in diff content and allocation failure handling.
🪄 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: 08b0c275-de83-434b-9fcf-5bd74a1b65b7
📒 Files selected for processing (2)
src/zsort.zigsrc/zsort_test.zig
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@AGENTS.md`:
- Line 9: The current command sequence in line 9 only restores the src fixture
if all commands succeed, and will discard any local edits if git checkout is
reached. Add a precondition to verify the tree is clean before running
validation, then install a shell cleanup trap that executes before the command
exits to restore the deliberately unsorted fixture regardless of whether the
validation commands succeed or fail. This ensures the fixture is always restored
after the complete build and import validation sequence runs, preventing
modifications from persisting if any validation step fails.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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`:
- Around line 869-883: Update the per-file loop around esc_path and the
banned_msg handling to conditionally free every escaped allocation before
continuing or leaving the iteration, mirroring formatUnifiedDiff’s cleanup
pattern. Ensure cleanup covers both escapeTerm(file_path) and the escaped
banned-pattern message. Add a regression test that fails before the cleanup and
detects retained allocations across affected files.
🪄 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: 43f4f8fd-4a37-4358-b29d-1edbaa083f64
📒 Files selected for processing (3)
AGENTS.mdsrc/zsort.zigsrc/zsort_test.zig
🚧 Files skipped from review as they are similar to previous changes (2)
- src/zsort_test.zig
- AGENTS.md
Summary by CodeRabbit
New Features
Documentation