Merge pull request #539 from dpatti/coffeescript-exec-args

Fix running coffee scripts with application --args

lib/config/exec.js

@@ -121,15 +121,18 @@ function exec(nodemonOptions, execMap) {
   // note: indexOf('coffee') handles both .coffee and .litcoffee
   if (!execDefined && options.exec === 'node' && scriptExt.indexOf('coffee') !== -1) {
     options.exec = 'coffee';
-    // ensure that we call: `coffee --nodejs ...`
-    if (options.execArgs === undefined) {
-      options.execArgs = [];
-    }
-    // if there's a leading argument to the exec that starts with `--` then
-    // it could be --debug or --debug-brk or something else intended for node
-    // so we'll add the --nodejs switch.
-    if ((options.args || []).join(' ').indexOf('--') === 0) {
-      options.execArgs.unshift('--nodejs');
+
+    // we need to get execArgs set before the script
+    // for example, in `nodemon --debug my-script.coffee --my-flag`, debug is an
+    // execArg, while my-flag is a script arg
+    var leadingArgs = (options.args || []).splice(0, options.scriptPosition);
+    options.execArgs = options.execArgs.concat(leadingArgs);
+    options.scriptPosition = 0;
+
+    if (options.execArgs.length > 0) {
+      // because this is the coffee executable, we need to combine the exec args
+      // into a single argument after the nodejs flag
+      options.execArgs = ['--nodejs', options.execArgs.join(' ')];
     }
   }
 

lib/config/index.js

@@ -60,7 +60,7 @@ config.load = function (settings, ready) {
     var cmd = command(config.options);
     config.command = {
       raw: cmd,
-      string: cmd.executable + (cmd.args.length ? ' ' + cmd.args.join(' ') : '')
+      string: utils.stringify(cmd.executable, cmd.args)
     };
 
     // now run automatic checks on system adding to the config object

lib/monitor/run.js

@@ -46,13 +46,6 @@ function run(options) {
     shFlag = '/c';
   }
 
-  var args = cmd.args.map(function (arg) {
-    if (arg.indexOf(' ') !== -1) {
-      arg = '"' + arg.replace(/"/g, '\\"') + '"';
-    }
-    return arg;
-  });
-
   var executable = cmd.executable;
 
   // special logic for windows, as spaces in the paths need the path fragment
@@ -61,7 +54,7 @@ function run(options) {
     executable = executable.replace(/\\((\w+\s+)+\w+)(?=([\\\.]))(?=([^"]*"[^"]*")*[^"]*$)/g, '\\"$1"');
   }
 
-  args = [executable].concat(args).join(' ').trim();
+  var args = utils.stringify(executable, cmd.args);
 
   var spawnArgs = [sh, [shFlag, args]];
 

lib/utils/index.js

@@ -49,6 +49,21 @@ var utils = module.exports = {
   },
   regexpToText: function (t) {
     return t.replace(/\.\*\\./g, '*.').replace(/\\{2}/g, '^^').replace(/\\/g, '').replace(/\^\^/g, '\\');
+  },
+  stringify: function (exec, args) {
+    // serializes an executable string and array of arguments into a string
+    args = args || [];
+
+    return [exec].concat(args.map(function(arg){
+      // if an argument contains a space, we want to show it with quotes around
+      // it to indicate that it is a single argument
+      if (arg.indexOf(' ') === -1) {
+        return arg;
+      } else {
+        // this should correctly escape nested quotes
+        return JSON.stringify(arg);
+      }
+    })).join(' ').trim();
   }
 };
 

test/cli/exec.test.js

@@ -2,7 +2,8 @@
 /*global describe:true, it: true */
 var exec = require('../../lib/config/exec'),
     command = require('../../lib/config/command'),
-    assert = require('assert');
+    assert = require('assert'),
+    utils = require('../../lib/utils');
 
 function toCmd(options) {
   var cmd = command({
@@ -12,7 +13,7 @@ function toCmd(options) {
 
   return {
     cmd: cmd,
-    string: cmd.executable + (cmd.args.length ? ' ' + cmd.args.join(' ') : '')
+    string: utils.stringify(cmd.executable, cmd.args)
   };
 }
 
@@ -87,7 +88,7 @@ describe('nodemon exec', function () {
     var options = exec({ script: 'app.coffee', nodeArgs: [ '--debug' ] });
 
     assert(options.exec.indexOf('coffee') === 0, 'using coffeescript to execute');
-    assert(options.execArgs.indexOf('--debug') !== -1);
+    assert(options.execArgs[1].indexOf('--debug') !== -1);
     assert(options.ext.indexOf('coffee') !== -1);
   });
 

test/cli/parse.test.js

@@ -4,7 +4,8 @@ var cli = require('../../lib/cli/'),
     exec = require('../../lib/config/exec'),
     pkg = require('../../package'),
     assert = require('assert'),
-    command = require('../../lib/config/command');
+    command = require('../../lib/config/command'),
+    utils = require('../../lib/utils');
 
 function asCLI(cmd) {
   return ('node nodemon ' + (cmd|| '')).trim();
@@ -28,7 +29,7 @@ function parse(cmd) {
 }
 
 function commandToString(command) {
-  return command.executable + (command.args.length ? ' ' + command.args.join(' ') : '');
+  return utils.stringify(command.executable, command.args);
 }
 
 describe('nodemon CLI parser', function () {
@@ -127,7 +128,7 @@ describe('nodemon CLI parser', function () {
 
     process.chdir(pwd);
 
-    assert(cmd === 'browserify --debug -t hbsfy app.js -o bundle.js', 'command is "' + cmd + '"');
+    assert(cmd === 'browserify --debug "-t hbsfy app.js -o bundle.js"', 'command is "' + cmd + '"');
   });
 
   it('should support spaces', function () {
@@ -282,6 +283,19 @@ describe('nodemon with CoffeeScript', function () {
     assert(settings.execOptions.execArgs.indexOf('--nodejs') === -1, 'is not using --nodejs');
   });
 
+  it('should not add --nodejs with app arguments', function () {
+    var settings = parse(asCLI('test/fixtures/app.coffee --my-app-arg'));
+    assert(settings.execOptions.exec.indexOf('coffee') === 0, 'executable is CoffeeScript');
+    assert(settings.execOptions.execArgs.indexOf('--nodejs') === -1, 'is not using --nodejs');
+  });
+
+  it('groups exec argument into a single --nodejs argument', function () {
+    var settings = parse(asCLI('--harmony --debug test/fixtures/app.coffee'));
+    assert(settings.execOptions.exec.indexOf('coffee') === 0, 'executable is CoffeeScript');
+    assert(settings.execOptions.execArgs[0] === '--nodejs', 'is using --nodejs');
+    assert(settings.execOptions.execArgs[1] === '--harmony --debug', 'is grouping exec arguments');
+  });
+
   it('should add --nodejs when used with --debug', function () {
     var settings = parse(asCLI('--debug test/fixtures/app.coffee'));
     var cmd = commandToString(command(settings));

test/config/load.test.js

@@ -16,7 +16,7 @@ function asCLI(cmd) {
 }
 
 function commandToString(command) {
-  return command.executable + (command.args.length ? ' ' + command.args.join(' ') : '');
+  return utils.stringify(command.executable, command.args);
 }
 
 describe('config load', function () {

test/events/scripts.test.js

@@ -21,7 +21,7 @@ function parse(cmd) {
 }
 
 function commandToString(command) {
-  return command.executable + (command.args.length ? ' ' + command.args.join(' ') : '');
+  return utils.stringify(command.executable, command.args);
 }
 
 describe('nodemon API events', function () {

test/utils/stringify.test.js

@@ -0,0 +1,31 @@
+'use strict';
+/*global describe:true, it: true */
+var stringify = require('../../lib/utils').stringify,
+    assert = require('assert');
+
+describe('stringify', function () {
+  it('should combine the executable and arguments', function () {
+    var string = stringify('node', ['./app.js', '--flag']);
+    var expected = 'node ./app.js --flag';
+
+    assert(string === expected, "stringified to " + string);
+  });
+  it('should not include excess whitespace', function () {
+    var string = stringify('node');
+    var expected = 'node';
+
+    assert(string === expected, "stringified to " + string);
+  });
+  it('should quote arguments with spaces', function () {
+    var string = stringify('node', ['./app.js', '--one --two']);
+    var expected = 'node ./app.js "--one --two"';
+
+    assert(string === expected, "stringified to " + string);
+  });
+  it('should escape quotes', function () {
+    var string = stringify('node', ['./app.js', '--one "--two --three"']);
+    var expected = 'node ./app.js "--one \\"--two --three\\""';
+
+    assert(string === expected, "stringified to " + string);
+  });
+});