Refactor arguments and options parsing

Now use minimist to match yo and fix type parsing.

Fix #821
Fix #813

Author: SBoudrias

lib/actions/help.js

@@ -51,10 +51,10 @@ help.help = function help() {
   return out.join('\n');
 };
 
-function formatArg(argItem) {
-  var arg = '<' + argItem.name + '>';
+function formatArg(config) {
+  var arg = '<' + config.name + '>';
 
-  if (!argItem.config.required) {
+  if (!config.required) {
     arg = '[' + arg + ']';
   }
 
@@ -101,12 +101,11 @@ help.desc = function desc(description) {
  * @returns {String} Text of options in formatted table
  */
 help.argumentsHelp = function argumentsHelp() {
-  var rows = this._arguments.map(function (arg) {
-    var config = arg.config;
+  var rows = this._arguments.map(function (config) {
 
     return [
       '',
-      arg.name ? arg.name : '',
+      config.name ? config.name : '',
       config.desc ? '# ' + config.desc : '',
       config.type ? 'Type: ' + config.type.name : '',
       'Required: ' + config.required

lib/index.js

@@ -8,7 +8,7 @@ var findUp = require('find-up');
 var readPkgUp = require('read-pkg-up');
 var chalk = require('chalk');
 var mkdirp = require('mkdirp');
-var nopt = require('nopt');
+var minimist = require('minimist');
 var runAsync = require('run-async');
 var through = require('through2');
 var userHome = require('user-home');
@@ -229,14 +229,6 @@ Base.prototype.option = function option(name, config) {
     this._options[name] = config;
   }
 
-  if (this.options[name] == null && config.alias) {
-    this.options[name] = this.options[config.alias];
-  }
-
-  if (this.options[name] == null) {
-    this.options[name] = config.default;
-  }
-
   this.parseOptions();
   return this;
 };
@@ -275,47 +267,76 @@ Base.prototype.argument = function argument(name, config) {
     type: String
   });
 
+  this._arguments.push(config);
 
-  var position = this._arguments.length;
-  this._arguments.push({
-    name: name,
-    config: config
-  });
-
-  Object.defineProperty(this, name, {
-    configurable: true,
-    enumerable: true,
-    get: function () {
-      // a bit of coercion and type handling, to be improved
-      // just dealing with Array/String, default is assumed to be String
-      var value = config.type === Array ? this.args.slice(position) : this.args[position];
-      return position >= this.args.length ? config.default : value;
-    },
-    set: function (value) {
-      this.args[position] = value;
-    }
-  });
-
-  this.checkRequiredArgs();
+  this.parseOptions();
   return this;
 };
 
 Base.prototype.parseOptions = function () {
-  var opts = {};
-  var shortOpts = {};
+  var minimistDef = {
+    string: [],
+    boolean: [],
+    alias: {},
+    default: {}
+  };
 
   _.each(this._options, function (option) {
-    opts[option.name] = option.type;
+    if (option.type === Boolean) {
+      minimistDef.boolean.push(option.name);
+    } else {
+      minimistDef.string.push(option.name);
+    }
 
     if (option.alias) {
-      shortOpts[option.alias] = '--' + option.name;
+      minimistDef.alias[option.alias] = option.name;
     }
+
+    // Only apply default values if we don't already have a value injected from
+    // the runner
+    if (option.name in this.options) {
+      minimistDef.default[option.name] = this.options[option.name];
+    } else if (option.alias && option.alias in this.options) {
+      minimistDef.default[option.alias] = this.options[option.alias];
+    } else if ('default' in option) {
+      minimistDef.default[option.name] = option.default;
+    }
+  }.bind(this));
+
+  var parsedOpts = minimist(this._args, minimistDef);
+
+  // Parse options to the desired type
+  _.each(parsedOpts, function (option, name) {
+    if (this._options[name] && option !== undefined) {
+      parsedOpts[name] = this._options[name].type(option);
+    }
+  }.bind(this));
+
+  // Parse positional arguments to valid options
+  this._arguments.forEach(function (config, index) {
+    var value;
+    if (index >= parsedOpts._.length) {
+      if ('default' in config) {
+        value = config.default;
+      } else {
+        return;
+      }
+    } else {
+      if (config.type === Array) {
+        value = parsedOpts._.slice(index, parsedOpts._.length);
+      } else {
+        value = config.type(parsedOpts._[index]);
+      }
+    }
+
+    parsedOpts[config.name] = value;
   });
 
-  opts = nopt(opts, shortOpts, this._args, 0);
-  _.extend(this.options, opts);
+  // Make the parsed options available to the instance
+  _.extend(this.options, parsedOpts);
+  this.args = this.arguments = parsedOpts._;
 
-  this.args = this.arguments = opts.argv.remain;
+  // Make sure required args are all present
   this.checkRequiredArgs();
 };
 
@@ -331,11 +352,11 @@ Base.prototype.checkRequiredArgs = function () {
     return;
   }
 
-  this._arguments.forEach(function (arg, position) {
+  this._arguments.forEach(function (config, position) {
     // If the help option was not provided, check whether the argument was
     // required, and whether a value was provided.
-    if (arg.config.required && position >= this.args.length) {
-      return this.emit('error', new Error('Did not provide required argument ' + chalk.bold(arg.name) + '!'));
+    if (config.required && position >= this.args.length) {
+      return this.emit('error', new Error('Did not provide required argument ' + chalk.bold(config.name) + '!'));
     }
   }, this);
 };

package.json

@@ -49,8 +49,8 @@
     "istextorbinary": "^2.1.0",
     "lodash": "^4.11.1",
     "mem-fs-editor": "^2.0.0",
+    "minimist": "^1.2.0",
     "mkdirp": "^0.5.0",
-    "nopt": "^3.0.0",
     "path-exists": "^2.0.0",
     "path-is-absolute": "^1.0.0",
     "pretty-bytes": "^3.0.1",

test/base.js

@@ -93,7 +93,9 @@ describe('Base', function () {
         resolved: 'test'
       });
 
+      generator.option('foo');
       assert.equal(generator.options.foo, true);
+      assert.deepEqual(generator.args, ['bar']);
     });
 
     it('set arguments based on nopt arguments', function () {
@@ -102,8 +104,9 @@ describe('Base', function () {
         resolved: 'test'
       });
 
+      generator.option('foo');
       generator.argument('baz');
-      assert.equal(generator.baz, 'bar');
+      assert.equal(generator.options.baz, 'bar');
     });
 
     it('set options with false values', function (done) {
@@ -546,18 +549,12 @@ describe('Base', function () {
 
     it('create the property specified with value from positional args', function () {
       this.dummy.argument('foo');
-      assert.equal(this.dummy.foo, 'bar');
-    });
-
-    it('can still be set as a property once defined', function () {
-      this.dummy.argument('foo');
-      this.dummy.foo = 'barbar';
-      assert.equal(this.dummy.foo, 'barbar');
+      assert.equal(this.dummy.options.foo, 'bar');
     });
 
     it('slice positional arguments when config.type is Array', function () {
       this.dummy.argument('bar', { type: Array });
-      assert.deepEqual(this.dummy.bar, ['bar', 'baz', 'bom']);
+      assert.deepEqual(this.dummy.options.bar, ['bar', 'baz', 'bom']);
     });
 
     it('raise an error if required arguments are not provided', function (done) {
@@ -593,7 +590,7 @@ describe('Base', function () {
       dummy.argument('baz');
       dummy.option('foo', { type: String });
 
-      assert.equal(dummy.baz, 'baz');
+      assert.equal(dummy.options.baz, 'baz');
     });
   });
 
@@ -621,7 +618,8 @@ describe('Base', function () {
           Base.apply(this, arguments);
 
           this.option('long-name', {
-            alias: 'short-name'
+            alias: 'short-name',
+            type: String
           });
         }
       });