Skip to content

Commit

Permalink
fix(parsing): Fixes negative numbers parsing as argument (#16)
Browse files Browse the repository at this point in the history
This fixes the way we parse process.argv and handle negative numbers. Negative numbers cannot be
used for options names from now on. Using micromist instead of minimist to resolve this problem.
Fixes #13.

#13
  • Loading branch information
mattallty authored Mar 6, 2017
1 parent f786ef5 commit 690b46d
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 19 deletions.
6 changes: 3 additions & 3 deletions lib/autocomplete.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"use strict";

const tabtab = require('tabtab');
const parseArgs = require('minimist');
const parseArgs = require('micromist');
const Promise = require('bluebird');

class Autocomplete {
Expand Down Expand Up @@ -174,10 +174,10 @@ class Autocomplete {
}

_complete(data, done) {

const args = parseArgs(data.args.slice(2));
const args = parseArgs(data.args.slice(1));
const cmd = args._.join(' ');
const currCommand = this._findCommand(cmd);

const realArgsCount = this._countArgs(currCommand, args);
const optionsAlreadyUsed = this._getOptionsAlreadyUsed(data.args);
const lastPartial = data.lastPartial;
Expand Down
9 changes: 5 additions & 4 deletions lib/program.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const GetterSetter = require('./utils').GetterSetter;
const Command = require('./command');
const parseArgs = require('minimist');
const parseArgs = require('micromist');
const Help = require('./help');
const path = require('path');
const Autocomplete = require('./autocomplete');
Expand Down Expand Up @@ -214,7 +214,7 @@ class Program extends GetterSetter {
const complete = new tabtabComplete({name: this.bin()});

if (typeof argv[1] !== "string" || this._supportedShell.indexOf(argv[1]) === -1) {
this.fatalError(new WrongNumberOfArgumentError(`Shell must be given (${this._supportedShell.join('/')})`, {}, this));
this.fatalError(new WrongNumberOfArgumentError(`A valid shell must be passed (${this._supportedShell.join('/')})`, {}, this));
} else {
this.logger().info(complete.script(this.bin(), this.bin(), argv[1].toLowerCase()));
}
Expand All @@ -226,14 +226,15 @@ class Program extends GetterSetter {
* @public
*/
parse(argv) {
argv = argv.slice(2);
const argvSlice = argv.slice(2);
const args = parseArgs(argv);
let options = Object.assign({}, args);
delete options._;

if (args._[0] === 'completion') {
return this._handleCompletionCommand(args, argv);
return this._handleCompletionCommand(args, argvSlice);
}

this._run(args._, options);
}

Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"engines": {
"node": ">= 4.4.5"
},
"homepage": "https:/mattallty/Caporal.js",
"keywords": [
"cli",
"argument-parser",
Expand Down Expand Up @@ -58,7 +59,7 @@
"cli-table2": "^0.2.0",
"fast-levenshtein": "^2.0.6",
"lodash.camelcase": "^4.3.0",
"minimist": "^1.2.0",
"micromist": "^1.0.1",
"prettyjson": "^1.2.1",
"tabtab": "^2.2.2",
"winston": "^2.3.1"
Expand Down
13 changes: 13 additions & 0 deletions tests/arg-validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,19 @@ describe("Argument validation", function() {
should(this.action.callCount).eql(1);
});

it(`should hanldle negative numbers in quoted arguments`, function() {
program
.command('order', 'Order something')
.argument('<what>', 'What to order', ["pizza", "burger", "smoothie"])
.argument('<how-much>', 'How much', program.INT)
.action(this.action);

program.parse(makeArgv(['order', "pizza", '-1']));
should(this.fatalError.callCount).eql(0);
should(this.action.callCount).eql(1);
should(this.action.args[0][0]).eql({ what: 'pizza', howMuch: -1 });
});

it(`should throw WrongNumberOfArgumentError when no argument is given to completion`, function() {
program.parse(makeArgv(['completion']));
should(this.fatalError.callCount).eql(1);
Expand Down
26 changes: 16 additions & 10 deletions tests/autocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ describe('Autocomplete', () => {
should(response).be.fulfilledWith([
'order:Order a pizza',
'return:Return an order'
]);
done();
]).then(_ => done()).catch(err => done(err));
//done();
});
});

Expand Down Expand Up @@ -127,8 +127,8 @@ describe('Autocomplete', () => {
should(response).be.Promise();
should(response).be.fulfilledWith([
'order:Order a pizza',
]);
done();
]).then(_ => done()).catch(err => done(err));

});
});

Expand All @@ -143,12 +143,18 @@ describe('Autocomplete', () => {
setImmediate(() => {
should(this._complete.called).be.true();
const response = this._complete.returnValues[0];
response.then(results => {
should(results).not.eql(['order:Order a pizza', 'return:Return an order']);
done();
}).catch(e => {
done(e);
});
should(response)
.be.fulfilledWith([
'margherita:Value for argument <kind>',
'hawaiian:Value for argument <kind>',
'fredo:Value for argument <kind>',
'--number:Number of pizza',
'--discount:Discount offer',
'--pay-by:Pay by option',
'-e:Add extra ingredients'
])
.then(_ => done())
.catch(err => done(err));
});

});
Expand Down
30 changes: 30 additions & 0 deletions tests/issues/issue-13-negative-num-as-arg.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/* global Program, logger, should, makeArgv, sinon */

const program = new Program();

program
.logger(logger)
.version('1.0.0')
.reset()
.command('solve', 'Solve quadratic')
.argument('<a>', 'A', program.INT)
.argument('<b>', 'B', program.INT)
.argument('<c>', 'C', program.INT)
.action(function(){});

describe("Issue #13 - Enter negative number as Argument", function() {

beforeEach(function () {
this.fatalError = sinon.stub(program, "fatalError");
});

afterEach(function () {
this.fatalError.restore();
program.reset();
});

it(`should not throw WrongNumberOfArgumentError with negative number as argument`, function() {
program.parse(makeArgv(['1', '2', '-3']));
should(this.fatalError.callCount).eql(0);
});
});
2 changes: 1 addition & 1 deletion tests/option-validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ describe('Passing a known short option', () => {
.action(function() {});

const error = sinon.stub(program, "fatalError");
program.parse(makeArgv('-t 278'));
program.parse(makeArgv(['-t', '278']));
should(error.callCount).be.eql(0);
error.restore();
program.reset();
Expand Down
6 changes: 6 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1932,6 +1932,12 @@ micromatch@^2.3.7:
parse-glob "^3.0.4"
regex-cache "^0.4.2"

micromist@^1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/micromist/-/micromist-1.0.1.tgz#5b497dd50098250a57ba2f355e9497d8e2394877"
dependencies:
lodash.camelcase "^4.3.0"

mime-db@~1.26.0:
version "1.26.0"
resolved "https://registry.yarnpkg.com/mime-db/-/mime-db-1.26.0.tgz#eaffcd0e4fc6935cf8134da246e2e6c35305adff"
Expand Down

0 comments on commit 690b46d

Please sign in to comment.