feat: add zsh support - #13
Conversation
|
Warning Review limit reached
More reviews will be available in 7 minutes and 21 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request adds zsh shell language support to the VS Code shell formatter extension. The changes register zsh as a language with the extension, bump the underlying shfmt tool to v3.13.1, implement file detection to pass the ChangesZsh Shell Support
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (3)
package.json (1)
125-133: ⚡ Quick winConsider adding other common zsh configuration files.
The current implementation includes
.zshrcand.zshenv, which are the most common zsh configuration files. However, zsh users also frequently use.zprofile,.zlogin, and.zlogoutfor shell initialization and cleanup.📝 Suggested enhancement
{ "id": "zsh", "extensions": [ ".zsh" ], "filenames": [ ".zshrc", - ".zshenv" + ".zshenv", + ".zprofile", + ".zlogin", + ".zlogout" ] }🤖 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 `@package.json` around lines 125 - 133, Update the "zsh" entry (id "zsh") in package.json to include additional common zsh config filenames by adding ".zprofile", ".zlogin", and ".zlogout" to the "filenames" array alongside the existing ".zshrc" and ".zshenv" so the "filenames" section covers all typical zsh init/cleanup files.src/shFormat.ts (1)
88-90: ⚡ Quick winConsider adding detection for other common zsh configuration files.
The current regex pattern matches
.zsh,.zshrc, and.zshenvfiles. To provide comprehensive zsh support, consider also detecting.zprofile,.zlogin, and.zlogoutfiles, which are commonly used zsh configuration files.Note: This suggestion corresponds to the optional enhancement suggested in
package.jsonlines 125-133. If you add those filenames to the language definition, this regex should also be updated.📝 Suggested enhancement
- if (/\.(zsh|zshrc|zshenv)$/.test(document.fileName)) { + if (/\.(zsh|zshrc|zshenv|zprofile|zlogin|zlogout)$/.test(document.fileName)) { shfmtFlags.push('--ln=zsh'); }🤖 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/shFormat.ts` around lines 88 - 90, The regex that detects zsh files in src/shFormat.ts currently only matches .zsh, .zshrc and .zshenv; update that pattern to also include .zprofile, .zlogin and .zlogout so the branch that pushes shfmtFlags.push('--ln=zsh') runs for those files too (e.g. change /\.(zsh|zshrc|zshenv)$/ to /\.(zsh|zshrc|zshenv|zprofile|zlogin|zlogout)$/). If you also add those filenames to the language definition in package.json, keep both places in sync.mise.toml (1)
2-2: 💤 Low valueConsider pinning node to a specific version for reproducibility.
Using
"latest"for the node version can lead to inconsistent development environments across team members and CI pipelines as the latest version changes over time.📌 Suggested change
-node = "latest" +node = "20" # or specify exact version like "20.15.0"🤖 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 `@mise.toml` at line 2, The node version is pinned to "latest" in the mise.toml entry node = "latest", which causes non-reproducible environments; replace that value with a specific, stable Node.js version (e.g., an LTS semver like "18.20.0" or the project-chosen version) and update any CI configs/README to match so local, CI, and team environments are consistent.
🤖 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.
Nitpick comments:
In `@mise.toml`:
- Line 2: The node version is pinned to "latest" in the mise.toml entry node =
"latest", which causes non-reproducible environments; replace that value with a
specific, stable Node.js version (e.g., an LTS semver like "18.20.0" or the
project-chosen version) and update any CI configs/README to match so local, CI,
and team environments are consistent.
In `@package.json`:
- Around line 125-133: Update the "zsh" entry (id "zsh") in package.json to
include additional common zsh config filenames by adding ".zprofile", ".zlogin",
and ".zlogout" to the "filenames" array alongside the existing ".zshrc" and
".zshenv" so the "filenames" section covers all typical zsh init/cleanup files.
In `@src/shFormat.ts`:
- Around line 88-90: The regex that detects zsh files in src/shFormat.ts
currently only matches .zsh, .zshrc and .zshenv; update that pattern to also
include .zprofile, .zlogin and .zlogout so the branch that pushes
shfmtFlags.push('--ln=zsh') runs for those files too (e.g. change
/\.(zsh|zshrc|zshenv)$/ to /\.(zsh|zshrc|zshenv|zprofile|zlogin|zlogout)$/). If
you also add those filenames to the language definition in package.json, keep
both places in sync.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0d18a1fa-c646-4672-a3b4-1a02f02a2f5c
📒 Files selected for processing (7)
README.mdmise.tomlpackage.jsonsrc/config.tssrc/shFormat.tstest/supported/.zshrctest/supported/test.zsh
🔗 Linked issue
#11
🧭 Context
Since v3.13.0, shfmt supports zsh.
📚 Description
Since v3.13.0, shfmt supports zsh.
Summary by CodeRabbit
New Features
Documentation
Chores