Conversation
POSIX echo can interpret backslash escapes and corrupt Windows-form paths in the generated shim header (pnpm/pnpm#14867). Print $link with printf '%s\n' so sed still converts backslashes. The Rust/pacquet copy of this header was fixed in pnpm/pnpm#14878. Co-authored-by: Bennett Payoyo <Yahiro025@users.noreply.github.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🔇 Additional comments (1)
📝 WalkthroughWalkthroughThe POSIX sh shim now uses ChangesPOSIX shim path handling
AppVeyor test configuration
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The shim change has targeted regression coverage, and AppVeyor is configured to run the tests. No actionable merge-blocking risk is indicated. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 3 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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. A rabbit checks the shim at night, Comment |
AppVeyor defaults to MSBuild and fails when the repository has no Visual Studio project. Install Node 22 and pnpm 12.4.1, then run pnpm test.
|
This package has been moved to the pnpm/pnpm monorepo. if you need to change it, make PRs to that repo. |
Summary
Fixes the POSIX bin shim header so Windows-form paths keep their backslashes until
sedconverts them.Related: pnpm/pnpm#14867. POSIX
echocan interpret\n/\t/ etc. beforesedruns, so a path likeC:\node_modules\.bin\tscbecomes corrupted on dash and macOS/bin/sh. pnpm 11 generates these shims via@zkochan/cmd-shim.The generated header now uses:
basedir=$(printf '%s\n' "$link" | command -p sed -e 's,\\,/,g')instead of
echo "$link". The commented documentation template above the live JS string is updated to match.The Rust/pacquet copy of this header was already fixed in pnpm/pnpm#14878. This is the remaining
@zkochan/cmd-shim/ pnpm 11 path.Test plan
npx tsc --build&&node --test test/test.js test/e2e.test.js(68 passed on Linux;/bin/shis dash)printf '%s\n' "$link"and notecho "$link"/bin/shturnsC:\node_modules\.bin\tscintoC:/node_modules/.bin/tscwithout injecting a newline or tabAI disclosure
I used Cursor to help draft the fix and tests. I reviewed the diff and verified the regression coverage before opening this PR.
Summary by CodeRabbit
Bug Fixes
\nand\tare preserved during path normalization.Tests