feat(env): add Nushell support via env.nu wrapper#1312
feat(env): add Nushell support via env.nu wrapper#1312naokihaba wants to merge 18 commits intovoidzero-dev:mainfrom
Conversation
✅ Deploy Preview for viteplus-preview canceled.
|
naokihaba
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d39efa0262
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| load-env ($exports | reduce -f {} {|it, acc| $acc | insert $it.key $it.value}) | ||
| } | ||
| let unsets = ($out | lines | where { $in =~ '^hide-env ' } | parse 'hide-env {key}' | get key) | ||
| for key in $unsets { hide-env $key } |
There was a problem hiding this comment.
vp env use mutations in emitted order
When vp env use is run without a version argument, the command intentionally prints hide-env VP_NODE_VERSION first and then $env.VP_NODE_VERSION = "..." so the final state is the resolved version. This wrapper parses all exports and applies them before all unsets, which reverses that ordering and leaves VP_NODE_VERSION unset in Nushell for the common vp env use flow. That breaks the new Nushell env-wrapper behavior because the shell no longer retains the resolved version after the command succeeds.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08a8e9de56
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| } else if config.nu_version.is_some() { | ||
| Shell::NuShell |
There was a problem hiding this comment.
Checking only NU_VERSION here causes false NuShell detection in POSIX wrappers when a bash/zsh session is launched from Nushell, because NU_VERSION is inherited by child processes. In that scenario vp env use is run with VP_ENV_USE_EVAL_ENABLE=1 from env, but this branch emits Nu syntax ($env.VP_NODE_VERSION = ...) that POSIX eval cannot execute, so the shell session is not updated. Detect NuShell via an explicit wrapper signal (or additional shell validation) instead of any inherited NU_VERSION.
Useful? React with 👍 / 👎.
| // Nushell env file with vp wrapper function. | ||
| // Completions delegate to Fish via vp.fish because clap_complete_nushell generates | ||
| // multiple rest params (e.g. for `vp install`), which Nushell does not support. | ||
| let env_nu_content = r#"# Vite+ environment setup (https://viteplus.dev) | ||
| $env.PATH = ($env.PATH | where { $in != "__VP_BIN__" } | prepend "__VP_BIN__") |
There was a problem hiding this comment.
ref: clap-rs/clap#5771
When a command has multiple Vec arguments (for example, when vp install has both packages and pass_through_args), clap_complete_nushell generates invalid syntax. This is because Nushell's extern only allows a single ...rest parameter
Error: nu::parser::multiple_rest_params
× Multiple rest params.
...packages: string
...pass_through_args: string ← Nushell rejects thisAs a workaround for this problem, completion processing is delegated to Fish. Running vp env setup generates vp.fish (which is shared with the Fish shell integration), so if Fish is installed, completions can be used without any additional configuration.
1 participant
resolves #1144
While comprehensive support for Nushell seems to be under development in #1305, that pull request has a large impact. Therefore, I have limited the scope of this change to env support to align with the existing issue.