Merge pull request #290 from isiahmeadows/pathext

feat(Windows): search PATHEXT instead of 3 hardcoded values

README.md

@@ -373,7 +373,8 @@ Examples:
 var nodeExec = which('node');
 ```
 
-Searches for `command` in the system's PATH. On Windows looks for `.exe`, `.cmd`, and `.bat` extensions.
+Searches for `command` in the system's PATH. On Windows, this uses the
+`PATHEXT` variable to append the extension if it's not already executable.
 Returns string containing the absolute path to the command.
 
 

src/which.js

@@ -2,6 +2,10 @@ var common = require('./common');
 var fs = require('fs');
 var path = require('path');
 
+// XP's system default value for PATHEXT system variable, just in case it's not
+// set on Windows.
+var XP_DEFAULT_PATHEXT = '.com;.exe;.bat;.cmd;.vbs;.vbe;.js;.jse;.wsf;.wsh';
+
 // Cross-platform method for splitting environment PATH variables
 function splitPath(p) {
   if (!p)
@@ -26,7 +30,8 @@ function checkPath(path) {
 //@ var nodeExec = which('node');
 //@ ```
 //@
-//@ Searches for `command` in the system's PATH. On Windows looks for `.exe`, `.cmd`, and `.bat` extensions.
+//@ Searches for `command` in the system's PATH. On Windows, this uses the
+//@ `PATHEXT` variable to append the extension if it's not already executable.
 //@ Returns string containing the absolute path to the command.
 function _which(options, cmd) {
   if (!cmd)
@@ -43,26 +48,37 @@ function _which(options, cmd) {
       if (where)
         return; // already found it
 
-      var attempt = path.resolve(dir + '/' + cmd);
+      var attempt = path.resolve(dir, cmd);
 
       if (common.platform === 'win') {
-        var baseAttempt = attempt;
-        attempt = baseAttempt + '.exe';
-        if (checkPath(attempt)) {
-          where = attempt;
-          return;
-        }
-        attempt = baseAttempt + '.bat';
-        if (checkPath(attempt)) {
-          where = attempt;
-          return;
+        attempt = attempt.toUpperCase();
+
+        // In case the PATHEXT variable is somehow not set (e.g.
+        // child_process.spawn with an empty environment), use the XP default.
+        var pathExtEnv = process.env.PATHEXT || XP_DEFAULT_PATHEXT;
+        var pathExtArray = splitPath(pathExtEnv.toUpperCase());
+        var i;
+
+        // If the extension is already in PATHEXT, just return that.
+        for (i = 0; i < pathExtArray.length; i++) {
+          var ext = pathExtArray[i];
+          if (attempt.slice(-ext.length) === ext && checkPath(attempt)) {
+            where = attempt;
+            return;
+          }
         }
-        attempt = baseAttempt + '.cmd';
-        if (checkPath(attempt)) {
-          where = attempt;
-          return;
+
+        // Cycle through the PATHEXT variable
+        var baseAttempt = attempt;
+        for (i = 0; i < pathExtArray.length; i++) {
+          attempt = baseAttempt + pathExtArray[i];
+          if (checkPath(attempt)) {
+            where = attempt;
+            return;
+          }
         }
       } else {
+        // Assume it's Unix-like
         if (checkPath(attempt)) {
           where = attempt;
           return;

test/which.js

@@ -16,15 +16,24 @@ shell.which();
 assert.ok(shell.error());
 
 var result = shell.which('asdfasdfasdfasdfasdf'); // what are the odds...
-assert.equal(shell.error(), null);
-assert.equal(result, null);
+assert.ok(!shell.error());
+assert.ok(!result);
 
 //
 // Valids
 //
 
-var result = shell.which('node');
-assert.equal(shell.error(), null);
-assert.equal(fs.existsSync(result), true);
+var node = shell.which('node');
+assert.ok(!shell.error());
+assert.ok(fs.existsSync(node));
+
+if (process.platform === 'win32') {
+    // This should be equivalent on Windows
+    var nodeExe = shell.which('node.exe');
+    assert.ok(!shell.error());
+    // If the paths are equal, then this file *should* exist, since that's
+    // already been checked.
+    assert.equal(node, nodeExe);
+}
 
 shell.exit(123);