Uh oh!
There was an error while loading. Please reload this page.
fix(completion): make install tips work on fresh systems - #206
Conversation
🦋 Changeset detectedLatest commit: ad9d2f3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
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 |
wyattjoh
left a comment
There was a problem hiding this comment.
The patch is directionally correct, but one user-facing install path is still left broken on fresh fish setups.
Uh oh!
There was an error while loading. Please reload this page.
The doctor zsh remedy told users to run `clerk completion zsh > ~/.zfunc/_clerk`, but `~/.zfunc` doesn't exist by default and the remedy also silently required `fpath`/`compinit` setup in ~/.zshrc. Lead with the `eval` form (zero-config) and point to `clerk completion --help` for the file-based method. Fix fish remedy to prefix `mkdir -p ~/.config/fish/completions`, and update the zsh script banner to include `mkdir -p ~/.zfunc` before the redirect.
The doctor remedy already prefixed the fish install command with mkdir -p, but the generated fish script banner, the --help tutorial text, and the README still showed the bare redirect which fails on a fresh system where ~/.config/fish/completions does not exist. Add the mkdir -p step to fish.ts banner, cli-program.ts help text, and README.md, mirroring what was already done for zsh. Add a test that verifies the fish banner includes mkdir before the write command.
46e7e7d to
ad9d2f3Compare
Summary
clerk doctorshowed a shell-completion remedy that blew up on a fresh system. The zsh tip wasclerk completion zsh > ~/.zfunc/_clerk, but~/.zfuncdoesn't exist by default, and even withmkdir -pthe user still neededfpath=(~/.zfunc $fpath)+autoload -Uz compinitin their~/.zshrc— which the remedy didn't mention. Lead with the zero-configeval "$(clerk completion zsh)"form and defer the file-install method toclerk completion --help.mkdir -p ~/.config/fish/completionsbefore redirecting, since that directory only exists if fish has been run at least once.mkdir -p ~/.zfuncbefore the redirect (so users debugging~/.zfunc/_clerksee the complete recipe).Before
After
Test plan
bun run test— passes (added regression tests for zsh/fish remedy wording and for the zsh bannermkdir -pordering)SHELL=/bin/zsh HOME=/tmp/empty bun run src/cli.ts doctor— zsh remedy now suggests the eval formSHELL=/usr/bin/fish HOME=/tmp/empty bun run src/cli.ts doctor— fish remedy includesmkdir -pbun run src/cli.ts completion zsh | head -20— banner comment tells users tomkdir -p ~/.zfuncbefore writing