Skip to content

fix(env): remove misleading PATH position check in vp env doctor#1140

Merged
fengmk2 merged 1 commit intovoidzero-dev:mainfrom
hamsurang:fix/doctor-path-position-check
Mar 25, 2026
Merged

fix(env): remove misleading PATH position check in vp env doctor#1140
fengmk2 merged 1 commit intovoidzero-dev:mainfrom
hamsurang:fix/doctor-path-position-check

Conversation

@jong-kyung
Copy link
Contributor

@jong-kyung jong-kyung commented Mar 25, 2026

Summary

  • Remove PATH position-based check in check_path() and replace with a simple presence check
  • Previously, vp env doctor warned "For best results, bin should be first in PATH" when ~/.vite-plus/bin was not at position 0 in PATH. On Windows, following this advice actually breaks vp commands due to the trampoline mechanism

Changes

  • paths.iter().position()paths.iter().any() (index no longer needed)
  • match 3-arm (Some(0), Some(pos), None) → if/else (present/absent)

Closes #1133

@netlify
Copy link

netlify bot commented Mar 25, 2026

Deploy Preview for viteplus-preview canceled.

Name Link
🔨 Latest commit f6f7962
🔍 Latest deploy log https://app.netlify.com/projects/viteplus-preview/deploys/69c373533abea80008f85c16

@jong-kyung
Copy link
Contributor Author

@codex review

@chatgpt-codex-connector
Copy link

Codex Review: Didn't find any major issues. Keep them coming!

ℹ️ 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".

@fengmk2
Copy link
Member

fengmk2 commented Mar 25, 2026

Could you enable Allow edits from maintainers on this PR? We need to rebase it onto main. Also, please rebase your branch onto the latest main. Thanks!

Replace the position-based PATH check with a simple presence check.
The old code warned when `~/.vite-plus/bin` was not first in PATH,
but on Windows putting bin first actually breaks `vp` due to the
trampoline mechanism. The per-tool shim resolution check already
provides meaningful diagnostics for shadowed shims.
@jong-kyung jong-kyung force-pushed the fix/doctor-path-position-check branch from 6397862 to f6f7962 Compare March 25, 2026 05:32
@fengmk2 fengmk2 merged commit 194225e into voidzero-dev:main Mar 25, 2026
23 checks passed
@jong-kyung
Copy link
Contributor Author

jong-kyung commented Mar 25, 2026

Could you enable Allow edits from maintainers on this PR? We need to rebase it onto main. Also, please rebase your branch onto the latest main. Thanks!

@fengmk2 Thanks for merging!

Just as a quick FYI, I couldn't enable the 'Allow edits' option earlier because I forked the repo to an organization account, which GitHub restricts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vp env doctor: Can't get rid of "note: For best results, bin should be first in PATH"

2 participants