Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default arguments to strings #449

Merged
merged 2 commits into from
Sep 10, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
372 changes: 233 additions & 139 deletions package-lock.json

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@
"moment": "^2.9.0",
"node-wifiscanner2": "^1.2.0",
"particle-api-js": "^6.4.1",
"particle-commands": "0.2.11",
"particle-library-manager": "0.1.12",
"particle-commands": "0.3.0",
"particle-library-manager": "0.1.13",
"request": "https://github.com/particle-iot/request/releases/download/v2.75.1-relativepath.1/request-2.75.1-relativepath.1.tgz",
"safe-buffer": "^5.1.1",
"semver": "^5.1.0",
Expand Down
25 changes: 23 additions & 2 deletions src/app/command-processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ class CLICommandItem {
configure(yargs, { options, setup, examples, version, epilogue }=this.buildOptions()) {
if (options) {
this.fetchAliases(options);
this.configureOptions(options);
// avoid converting positional arguments to numbers by default
const optionsWithDefaults = Object.assign({ '_': { string: true } }, options);
yargs.options(optionsWithDefaults);
Expand Down Expand Up @@ -146,6 +147,26 @@ class CLICommandItem {
});
}

configureOptions(options) {
Object.keys(options).forEach((key) => {
const option = options[key];
if (!option.hasOwnProperty('nargs') &&
!option.boolean &&
!option.count &&
!option.array) {
option.nargs = 1;
}

if (!option.hasOwnProperty('number') &&
!option.hasOwnProperty('boolean') &&
!option.hasOwnProperty('string') &&
!option.hasOwnProperty('count') &&
!option.hasOwnProperty('array')) {
option.string = true;
}
});
}

/**
* Finds the original name of an option given a possible alias.
* @param {string} name The option name to unalias.
Expand Down Expand Up @@ -428,7 +449,7 @@ function consoleErrorLogger(console, yargs, exit, err) {
// Adapted from: https://github.com/bcoe/yargs/blob/master/lib/validation.js#L83-L110
function checkForUnknownArguments(yargs, argv, command) {
const aliasLookup = {};
const descriptions = yargs.getUsageInstance().getDescriptions();
const flags = yargs.getOptions().key;
const demanded = yargs.getDemanded();
const unknown = [];

Expand All @@ -440,8 +461,8 @@ function checkForUnknownArguments(yargs, argv, command) {

function isUnknown(key) {
return (key !== '$0' && key !== '_' && key !== 'params' &&
!descriptions.hasOwnProperty(key) &&
!demanded.hasOwnProperty(key) &&
!flags.hasOwnProperty(key) &&
!aliasLookup.hasOwnProperty('no-' + key) &&
!aliasLookup.hasOwnProperty(key));
}
Expand Down
18 changes: 6 additions & 12 deletions src/cli/cloud.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ export default ({ commandProcessor, root }) => {

const compileOptions = {
'target': {
description: 'The firmware version to compile against. Defaults to latest version, or version on device for cellular.',
nargs: 1
description: 'The firmware version to compile against. Defaults to latest version, or version on device for cellular.'
}
};

Expand Down Expand Up @@ -80,8 +79,7 @@ export default ({ commandProcessor, root }) => {
params: '<deviceType> [files...]',
options: Object.assign({}, compileOptions, {
'saveTo': {
description: 'Filename for the compiled binary',
nargs: 1
description: 'Filename for the compiled binary'
}
}),
handler: (args) => {
Expand Down Expand Up @@ -113,22 +111,18 @@ export default ({ commandProcessor, root }) => {
options: {
u: {
description: 'your username',
alias: 'username',
nargs: 1
alias: 'username'
},
p: {
description: 'your password',
alias: 'password',
nargs: 1
alias: 'password'
},
t: {
description: 'an existing Particle access token to use',
alias: 'token',
nargs: 1
alias: 'token'
},
otp: {
description: 'the login code if two-step authentication is enabled',
nargs: 1
description: 'the login code if two-step authentication is enabled'
}
},
handler: (args) => {
Expand Down
1 change: 1 addition & 0 deletions src/cli/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export default ({ commandProcessor, root }) => {
params: '[profile] [setting] [value]',
options: {
'list': {
boolean: true,
description: 'Display available configurations'
}
},
Expand Down
6 changes: 2 additions & 4 deletions src/cli/flash.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,10 @@ export default ({ commandProcessor, root }) => {
description: 'Answer yes to all questions'
},
'target': {
description: 'The firmware version to compile against. Defaults to latest version, or version on device for cellular.',
nargs: 1
description: 'The firmware version to compile against. Defaults to latest version, or version on device for cellular.'
},
'port': {
describe: 'Use this serial port instead of auto-detecting. Useful if there are more than 1 connected device. Only available for serial',
nargs: 1
describe: 'Use this serial port instead of auto-detecting. Useful if there are more than 1 connected device. Only available for serial'
}
},
handler: (args) => {
Expand Down
13 changes: 5 additions & 8 deletions src/cli/keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ export default ({ commandProcessor, root }) => {

const protocolOption = {
'protocol': {
description: 'Communication protocol for the device using the key. tcp or udp',
nargs: 1
description: 'Communication protocol for the device using the key. tcp or udp'
}
};

Expand Down Expand Up @@ -45,8 +44,7 @@ export default ({ commandProcessor, root }) => {
options: {
'product_id': {
number: true,
description: 'The product ID to use when provisioning a new device',
nargs: 1
description: 'The product ID to use when provisioning a new device'
}
},
handler: (args) => {
Expand All @@ -69,12 +67,11 @@ export default ({ commandProcessor, root }) => {
params: '[filename]',
options: Object.assign({}, protocolOption, {
'host': {
description: 'Hostname or IP address of the server to add to the key',
nargs: 1
description: 'Hostname or IP address of the server to add to the key'
},
'port': {
description: 'Port number of the server to add to the key',
nargs: 1
number: true,
description: 'Port number of the server to add to the key'
}
}),
handler: (args) => {
Expand Down
23 changes: 8 additions & 15 deletions src/cli/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ export default ({ commandProcessor, root }) => {
const lib = commandProcessor.createCategory(root, 'library', 'Manage firmware libraries', { alias: 'libraries' });

commandProcessor.createCommand(lib, 'add', 'Add a library to the current project.', {
options: {},
params: '<name>',
handler: (...args) => require('./library_add').command(api(), ...args),
examples: {
Expand All @@ -27,16 +26,13 @@ export default ({ commandProcessor, root }) => {
commandProcessor.createCommand(lib, 'create', 'Create a new library in the specified or current directory', {
options: {
'name': {
description: 'The name of the library to create.',
nargs: 1
description: 'The name of the library to create.'
},
'version': {
description: 'The initial version of the library to create.',
nargs: 1
description: 'The initial version of the library to create.'
},
'author': {
description: 'The author of the library.',
nargs: 1
description: 'The author of the library.'
}
},
handler: (...args) => require('./library_init').command(...args)
Expand All @@ -58,7 +54,6 @@ export default ({ commandProcessor, root }) => {
alias: 'y'
},
'dest': {
boolean: false,
description: 'the directory to install to'
}
},
Expand All @@ -74,20 +69,19 @@ export default ({ commandProcessor, root }) => {
commandProcessor.createCommand(lib, 'list', 'List libraries available', {
options: {
'filter': {
description: 'filters libraries not matching the text',
nargs: 1
description: 'filters libraries not matching the text'
},
'non-interactive': {
boolean: true,
description: 'Prints a single page of libraries without prompting'
},
'page': {
description: 'Start the listing at the given page number',
nargs: 1
number: true,
description: 'Start the listing at the given page number'
},
'limit': {
description: 'The number of items to show per page',
nargs: 1
number: true,
description: 'The number of items to show per page'
}
},
params: '[sections...]',
Expand Down Expand Up @@ -128,7 +122,6 @@ export default ({ commandProcessor, root }) => {
});

commandProcessor.createCommand(lib, 'publish', 'Publish a private library, making it public', {
options: {},
params: '[name]',
handler: (...args) => require('./library_publish').command(api(), ...args)
});
Expand Down
3 changes: 1 addition & 2 deletions src/cli/project.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ export default ({ commandProcessor, root }) => {
commandProcessor.createCommand(project, 'create', 'Create a new project in the current or specified directory', {
options: {
'name' : {
description: 'provide a name for the project',
nargs: 1
description: 'provide a name for the project'
}
},
params: '[dir]',
Expand Down
6 changes: 2 additions & 4 deletions src/cli/serial.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ export default ({ commandProcessor, root }) => {

const portOption = {
'port': {
describe: 'Use this serial port instead of auto-detecting. Useful if there are more than 1 connected device',
nargs: 1
describe: 'Use this serial port instead of auto-detecting. Useful if there are more than 1 connected device'
}
};

Expand Down Expand Up @@ -41,8 +40,7 @@ export default ({ commandProcessor, root }) => {
commandProcessor.createCommand(serial, 'wifi', 'Configure Wi-Fi credentials over serial', {
options: Object.assign({
'file': {
description: 'Take the credentials from a JSON file instead of prompting for them',
nargs: 1
description: 'Take the credentials from a JSON file instead of prompting for them'
}
}, portOption),
handler: (args) => {
Expand Down
3 changes: 1 addition & 2 deletions src/cli/subscribe.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ export default ({ commandProcessor, root }) => {
description: 'Listen to all events instead of just those from my devices'
},
'device': {
describe: 'Listen to events from this device only',
nargs: 1
describe: 'Listen to events from this device only'
}
},
handler: (args) => {
Expand Down
41 changes: 38 additions & 3 deletions test/app/command-processor.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ describe('command-line parsing', () => {
it('can accept options', () => {
// see '.choices()` https://www.npmjs.com/package/yargs
const app = commandProcessor.createAppCategory({ options: {
cm: { alias: 'chundermonkey' }
cm: { alias: 'chundermonkey', boolean: true }
}});

expect(commandProcessor.parse(app, ['--cm'])).to.have.property('cm').equal(true);
Expand All @@ -177,10 +177,10 @@ describe('command-line parsing', () => {
it('refuses options meant for other commands', () => {
const app = commandProcessor.createAppCategory();
const one = commandProcessor.createCommand(app, 'one', 'first', { options: {
one: { alias: '1', description: ''}
one: { alias: '1', description: '', boolean: true }
}});
const two = commandProcessor.createCommand(app, 'two', 'second', { options: {
two: { alias: '2', description: ''}
two: { alias: '2', description: '', boolean: true }
}});

// sanity test
Expand All @@ -203,6 +203,17 @@ describe('command-line parsing', () => {
test: {
alias: 'flag',
boolean: true
},
string: {
},
multipleStrings: {
nargs: 2
},
number: {
number: true,
},
count: {
count: true
}
}
});
Expand Down Expand Up @@ -312,6 +323,30 @@ describe('command-line parsing', () => {
.deep.equal(commandProcessor.errors.unknownParametersError(['stragglers', 'here']));
});

it('flags default to strings', () => {
const result = paramsCommand('', ['--string', '42']);
expect(result).to.have.property('string').equal('42');
});

it('flags require one argument by default', () => {
expect(() => paramsCommand('', ['--string'])).to.throw(/Not enough arguments following: string/);
});

it('flags can require multiple arguments', () => {
const result = paramsCommand('', ['--multipleStrings', '1', '2']);
expect(result).to.have.property('multipleStrings').deep.equal(['1', '2']);
});

it('number flag get converted', () => {
const result = paramsCommand('', ['--number', '42']);
expect(result).to.have.property('number').equal(42);
});

it('count flag get counted', () => {
const result = paramsCommand('', ['--count', '--count']);
expect(result).to.have.property('count').equal(2);
});

it('keeps device ids as strings', () => {
const result = paramsCommand('<deviceid>', ['500000000000000000000000']).params;
expect(result).to.have.property('deviceid').equal('500000000000000000000000');
Expand Down