Skip to content

refactor: replace spawn('which') with 'which' npm package#75

Closed
marcusolsson wants to merge 1 commit into
mainfrom
fix/use-which-package
Closed

refactor: replace spawn('which') with 'which' npm package#75
marcusolsson wants to merge 1 commit into
mainfrom
fix/use-which-package

Conversation

@marcusolsson

Copy link
Copy Markdown
Contributor

Summary

Replace the manual spawn('which', [binary]) approach in SpawnCommandRunner.checkInstalled() with the cross-platform which npm package.

Changes

  • src/commands/code/adapters/spawn-command-runner.ts: Replaced child-process-based which check with the which library
  • package.json: Added which as a dependency and @types/which as a dev dependency

Benefits

  • Cross-platform: Works on Windows (where.exe) and Unix (which) without manual platform detection
  • No child process overhead: Simple PATH lookup without spawning a subprocess
  • Cleaner error handling: try/catch instead of exit code checking

Verification

  • ✅ TypeScript check passes (tsc --noEmit)
  • ✅ All 283 tests pass (vitest run)

Replace the manual spawn('which', [binary]) approach in SpawnCommandRunner
checkInstalled() with the cross-platform 'which' package.

Benefits:
- Cross-platform: works on Windows (where.exe) and Unix (which)
- No child process overhead for simple PATH lookups
- Cleaner error handling (try/catch vs exit code checking)
@marcusolsson

Copy link
Copy Markdown
Contributor Author

Merged manually via fast-forward. The changes from this PR (plus a compatibility fix downgrading which from v6 to v5 for Node >=20.0.0 support) are now on main.

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.

1 participant