Skip to content

Search PATHEXT instead of 3 hardcoded values#134

Closed
dead-claudia wants to merge 2 commits intoshelljs:masterfrom
dead-claudia:patch-1
Closed

Search PATHEXT instead of 3 hardcoded values#134
dead-claudia wants to merge 2 commits intoshelljs:masterfrom
dead-claudia:patch-1

Conversation

@dead-claudia
Copy link
Copy Markdown

Better to search the PATHEXT variable instead of hard-coding an incomplete list for Windows executables. A default fallback consisting of XP's default value is also introduced just in case. The fallback value is hardcoded as a constant at the top of the file.

impinball added 2 commits June 15, 2014 17:20
Better to search the PATHEXT variable instead of hard-coding an incomplete list for Windows executables. A default fallback consisting of XP's default value is also introduced just in case. The fallback value is hardcoded as a constant at the top of the file.

It also results in simpler code.
@arturadib
Copy link
Copy Markdown
Collaborator

Hi- thanks for the patch. Alas I no longer have a Windows machine to test this on and Travis doesn't support Windows, can someone please confirm the tests pass? Thanks!

@nfischer
Copy link
Copy Markdown
Member

@isiahmeadows would you be able to rebase this off master? This might be a valuable addition, since we ran into that issue in #239 (and the PATHEXT variable was brought to my attention). @BYK could you review?

@nfischer nfischer added fix Bug/defect, or a fix for such a problem low priority Windows labels Jan 14, 2016
dead-claudia pushed a commit to dead-claudia/shelljs that referenced this pull request Jan 14, 2016
@dead-claudia
Copy link
Copy Markdown
Author

Closed this and redid it in #290.

Problem was that I have changed my username since I made the original PR.

dead-claudia pushed a commit to dead-claudia/shelljs that referenced this pull request Jan 29, 2016
Redo of shelljs#134

`which` now searches through PATHEXT on Windows, and it also now does a
case-insensitive comparison. This better fits the Windows environment, where
the OS usually ignores case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Bug/defect, or a fix for such a problem low priority Windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants