Repository navigation
fix(env): call fish built-ins with builtin in env.fish - #3353
Conversation
User functions named like fish built-ins replaced them in env.fish. A user count function made vp env use report success without setting VP_NODE_VERSION. Closes voidzero-dev#3128
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Maybe we should wait for #3354 to be merged first and then do a final check on this PR just to be safe. |
|
I merged All 3 I also ran a manual check with the debug
$ builtin source <tmp>/env.fish
-> status=0
$ vp env use 20.20.2 --no-install
Using Node.js v20.20.2 (resolved from 20.20.2)
-> status=0
VP_NODE_VERSION=20.20.2
$ vp env use --help >/dev/null
-> status=0
VP_NODE_VERSION=20.20.2 (expected: unchanged)
$ vp -C . env use 22.23.4 --no-install
Using Node.js v22.23.4 (resolved from 22.23.4)
-> status=0
VP_NODE_VERSION=22.23.4
$ complete -C 'vpr '
build vite build
test:e2e playwright test
test:unit vitest run
$ complete -C 'vpr test\:u'
test:unit vitest run
$ count a b c
0 |
Context
When a user defines a fish function with the name of a built-in,
env.fishcalls the user function instead of the built-in. With a usercountfunction,vp env usereports success, but the Node.js version does not change.The fish documentation for
functionwarns about this case. It says that a function with the name of a built-in often causes problems "in scripts written by others".env.fishis such a script.This PR adds
builtinto each built-in thatenv.fishcalls:count,math,contains,complete,commandline, andsource.builtinskips functions and runs only the built-in. #3123 did the same for Nushell with the%sigil.This PR does not change the
vpcompletion. clap_complete registers it with barecompleteandcommandline, soenv.fishcannot protect it. fish's own completions also call barecommandline.