Fix #667 - use which's result#668
Conversation
|
Fixes #750 |
There was a problem hiding this comment.
What does this fix? If which('python') succeeds, then by definition python is on the PATH, right?
EDIT: Let me rephrase. #750 is about batch files. How does using execPath help here?
There was a problem hiding this comment.
the which module is more similar to windows PATH resolution semantics then the following require('child_process').execFile in line 85.
execFile fails to resolve *.bat and *.cmd files from the PATH, but will run them if explicitly specified to.
There was a problem hiding this comment.
Can you add a comment explaining that (Properly capitalized and punctuated, please.)?
It would be nice to have some kind of regression test for this. Maybe you can module.exports.test.checkPython = checkPython and verify in the test that executing the result with -V prints the python version banner?
There was a problem hiding this comment.
I went ahead and filed a PR that adds a regression test: #756
|
Added comment. |
Break out the search logic into a separate function and add a regression test. References: #668 PR-URL: #756 Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Fixes: #667 Fixes: #750 PR-URL: #668 Reviewed-By: Ben Noordhuis <[email protected]>
|
Thanks, landed in 268f1ca. |
2 participants
No description provided.