Skip to content

Commit cd091f6

Browse files
bjornstarbjornstar
committed
Improve command line argument parsing (Fixes #258)
1 parent 05aacd5 commit cd091f6

3 files changed

Lines changed: 206 additions & 68 deletions

File tree

‎lib/cli.js‎

Lines changed: 57 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,84 +1,78 @@
1+
const assert = require('assert');
12
const minimist = require('minimist');
23
const { resolve } = require('path');
34

4-
const { defaultConfig, getConfig } = require('./cfg');
5+
const { getConfig } = require('./cfg');
56

6-
const configKeys = Object.keys(defaultConfig);
7+
const arrayify = v => (Array.isArray(v) ? [...v] : [v]);
8+
const argify = key => ({ arg: `--${key}`, key });
79

8-
function resolvePath(unresolvedPath) {
9-
return resolve(process.cwd(), unresolvedPath);
10-
}
10+
const resolvePath = p => resolve(process.cwd(), p);
1111

12-
const doubleDash = s => /^--/.test(s);
13-
const dash = s => /^-[^-]*$/.test(s);
12+
const nodeAlias = { require: 'r' };
13+
const nodeBoolean = ['expose_gc'];
14+
const nodeOptional = ['inspect', 'inspect-brk'];
15+
const nodeString = ['require'];
1416

15-
function getFirstNonOptionArgIndex(args) {
16-
for (let i = 2; i < args.length; i += 1) {
17-
if (!doubleDash(args[i]) && !dash(args[i]) && !dash(args[i - 1] || '')) return i;
18-
}
17+
const nodeDevBoolean = ['clear', 'dedupe', 'fork', 'notify', 'poll', 'respawn', 'vm'];
18+
const nodeDevNumber = ['debounce', 'deps', 'interval'];
19+
const nodeDevString = ['graceful_ipc', 'ignore', 'timestamp'];
1920

20-
return args.length - 1;
21-
}
21+
const alias = Object.assign({}, nodeAlias);
22+
const boolean = [...nodeBoolean, ...nodeDevBoolean];
23+
const string = [...nodeString, ...nodeDevString];
2224

23-
function unique(k) {
24-
const seen = [];
25-
return o => {
26-
if (!seen.includes(o[k])) {
27-
seen.push(o[k]);
28-
return true;
29-
}
30-
return false;
31-
};
32-
}
25+
const nodeArgsReducer = opts => (out, { arg, key }) => {
26+
const value = opts[key];
3327

34-
module.exports = argv => {
35-
const unknownArgs = [];
28+
if (typeof value === 'boolean') {
29+
value && out.push(arg);
30+
} else if (typeof value !== 'undefined') {
31+
arrayify(value).forEach(v => {
32+
if (arg.includes('=')) {
33+
out.push(`${arg.split('=')[0]}=${v}`);
34+
} else {
35+
out.push(`${arg}=${v}`);
36+
}
37+
});
38+
}
39+
40+
delete opts[key];
41+
42+
return out;
43+
};
3644

37-
const scriptIndex = getFirstNonOptionArgIndex(argv);
45+
const nodeOptionalFactory = args => arg => {
46+
const isNodeOptional = nodeOptional.includes(arg.substring(2));
47+
if (isNodeOptional) args.push(arg);
48+
return !isNodeOptional;
49+
};
3850

39-
const script = argv[scriptIndex];
40-
const scriptArgs = argv.slice(scriptIndex + 1);
41-
const devArgs = argv.slice(2, scriptIndex);
51+
const unknownFactory = args => arg => {
52+
const [, key] = Object.keys(minimist([arg]));
53+
key && !nodeDevNumber.includes(key) && args.push({ arg, key });
54+
};
4255

43-
const opts = minimist(devArgs, {
44-
boolean: ['clear', 'dedupe', 'fork', 'notify', 'poll', 'respawn', 'vm'],
45-
string: ['graceful_ipc', 'ignore', 'timestamp'],
46-
default: getConfig(script),
47-
unknown: arg => {
48-
const key = Object.keys(minimist([arg]))[1];
56+
module.exports = argv => {
57+
const nodeOptionalArgs = [];
58+
const args = argv.slice(2).filter(nodeOptionalFactory(nodeOptionalArgs));
4959

50-
if (!configKeys.includes(key)) {
51-
unknownArgs.push({ arg, key });
52-
}
53-
}
54-
});
60+
const unknownArgs = [];
61+
const unknown = unknownFactory(unknownArgs);
5562

56-
const nodeArgs = unknownArgs.filter(unique('key')).reduce((out, { arg, key }) => {
57-
const value = opts[key];
63+
const {
64+
_: [script, ...scriptArgs]
65+
} = minimist(args, { alias, boolean, string, unknown });
5866

59-
if (typeof value !== 'boolean' && !arg.includes('=')) {
60-
if (Array.isArray(value)) {
61-
value.forEach(v => out.push(arg, v));
62-
} else {
63-
out.push(arg, value);
64-
}
65-
} else {
66-
out.push(arg);
67-
}
67+
assert(script, 'Could not parse command line arguments');
6868

69-
return out;
70-
}, []);
69+
const opts = minimist(args, { alias, boolean, default: getConfig(script) });
7170

72-
unknownArgs.forEach(({ key }) => {
73-
delete opts[key];
74-
});
71+
const nodeArgs = [...nodeBoolean.map(argify), ...nodeString.map(argify), ...unknownArgs]
72+
.sort((a, b) => a.key - b.key)
73+
.reduce(nodeArgsReducer(opts), [...nodeOptionalArgs]);
7574

76-
opts.ignore = [...(Array.isArray(opts.ignore) ? opts.ignore : [opts.ignore])].map(resolvePath);
75+
opts.ignore = arrayify(opts.ignore).map(resolvePath);
7776

78-
return {
79-
script,
80-
scriptArgs,
81-
nodeArgs,
82-
opts
83-
};
77+
return { nodeArgs, opts, script, scriptArgs };
8478
};

‎package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,14 @@
3535
"dateformat": "^3.0.3",
3636
"dynamic-dedupe": "^0.3.0",
3737
"filewatcher": "~3.0.0",
38-
"minimist": "^1.1.3",
38+
"minimist": "^1.2.5",
3939
"node-notifier": "^8.0.1",
4040
"resolve": "^1.0.0",
4141
"semver": "^7.3.5"
4242
},
4343
"devDependencies": {
4444
"@types/node": "^14.14.37",
45-
"eslint": "^7.23.0",
45+
"eslint": "^7.25.0",
4646
"eslint-plugin-import": "^2.22.1",
4747
"husky": "^6.0.0",
4848
"lint-staged": "^10.5.4",

‎test/cli.js‎

Lines changed: 147 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,19 +68,19 @@ tap.test('cli overrides .node-dev.json from false to true', t => {
6868
tap.test('-r ts-node/register --inspect test/fixture/server.js', t => {
6969
const argv = 'node bin/node-dev -r ts-node/register --inspect test/fixture/server.js'.split(' ');
7070
const { nodeArgs } = cli(argv);
71-
t.same(nodeArgs, ['-r', 'ts-node/register', '--inspect']);
71+
t.same(nodeArgs, ['--inspect', '--require=ts-node/register']);
7272
t.end();
7373
});
7474

7575
tap.test('--inspect -r ts-node/register test/fixture/server.js', t => {
7676
const argv = 'node bin/node-dev --inspect -r ts-node/register test/fixture/server.js'.split(' ');
7777
const { nodeArgs } = cli(argv);
78-
t.same(nodeArgs, ['--inspect', '-r', 'ts-node/register']);
78+
t.same(nodeArgs, ['--inspect', '--require=ts-node/register']);
7979
t.end();
8080
});
8181

8282
tap.test('--expose_gc gc.js foo', t => {
83-
const argv = 'node bin/node-dev --expose_gc test/fixture/gc.js test/fixture/foo'.split(' ');
83+
const argv = 'node bin/node-dev --expose_gc test/fixture/gc.js foo'.split(' ');
8484
const { nodeArgs } = cli(argv);
8585
t.same(nodeArgs, ['--expose_gc']);
8686
t.end();
@@ -139,3 +139,147 @@ tap.test('--debounce=2000', t => {
139139
t.equal(debounce, 2000);
140140
t.end();
141141
});
142+
143+
tap.test('--require source-map-support/register', t => {
144+
const { nodeArgs } = cli([
145+
'node',
146+
'bin/node-dev',
147+
'--require',
148+
'source-map-support/register',
149+
'test'
150+
]);
151+
152+
t.same(nodeArgs, ['--require=source-map-support/register']);
153+
t.end();
154+
});
155+
156+
tap.test('--require=source-map-support/register', t => {
157+
const { nodeArgs } = cli([
158+
'node',
159+
'bin/node-dev',
160+
'--require=source-map-support/register',
161+
'test'
162+
]);
163+
164+
t.same(nodeArgs, ['--require=source-map-support/register']);
165+
t.end();
166+
});
167+
168+
tap.test('-r source-map-support/register', t => {
169+
const { nodeArgs } = cli(['node', 'bin/node-dev', '-r', 'source-map-support/register', 'test']);
170+
171+
t.same(nodeArgs, ['--require=source-map-support/register']);
172+
t.end();
173+
});
174+
175+
tap.test('-r=source-map-support/register', t => {
176+
const { nodeArgs } = cli(['node', 'bin/node-dev', '-r=source-map-support/register', 'test']);
177+
178+
t.same(nodeArgs, ['--require=source-map-support/register']);
179+
t.end();
180+
});
181+
182+
tap.test('--inspect=127.0.0.1:12345', t => {
183+
const { nodeArgs } = cli(['node', 'bin/node-dev', '--inspect=127.0.0.1:12345', 'test']);
184+
185+
t.same(nodeArgs, ['--inspect=127.0.0.1:12345']);
186+
t.end();
187+
});
188+
189+
tap.test('--inspect', t => {
190+
const { nodeArgs } = cli(['node', 'bin/node-dev', '--inspect', 'test']);
191+
192+
t.same(nodeArgs, ['--inspect']);
193+
t.end();
194+
});
195+
196+
tap.test('--require source-map-support/register --require ts-node/register', t => {
197+
const { nodeArgs } = cli([
198+
'node',
199+
'bin/node-dev',
200+
'--require',
201+
'source-map-support/register',
202+
'--require',
203+
'ts-node/register',
204+
'test'
205+
]);
206+
207+
t.same(nodeArgs, ['--require=source-map-support/register', '--require=ts-node/register']);
208+
t.end();
209+
});
210+
211+
// This should display usage information at some point
212+
tap.test('No script or option should fail', t => {
213+
t.throws(() => cli(['node', 'bin/node-dev']));
214+
t.end();
215+
});
216+
217+
tap.test('Just an option should fail', t => {
218+
t.throws(() => cli(['node', 'bin/node-dev', '--option']));
219+
t.end();
220+
});
221+
222+
tap.test('Just an option with a value should fail', t => {
223+
t.throws(() => cli(['node', 'bin/node-dev', '--option=value']));
224+
t.end();
225+
});
226+
227+
tap.test('An unknown argument with a value instead of a script should fail.', t => {
228+
t.throws(() => cli(['node', 'bin/node-dev', '--unknown-arg', 'value']));
229+
t.end();
230+
});
231+
232+
tap.test('An unknown argument with a value', t => {
233+
const { nodeArgs } = cli(['node', 'bin/node-dev', '--unknown-arg=value', 'test']);
234+
235+
t.same(nodeArgs, ['--unknown-arg=value']);
236+
t.end();
237+
});
238+
239+
tap.test('An unknown argument without a value can use -- to delimit', t => {
240+
// use -- to delimit the end of options
241+
const { nodeArgs } = cli(['node', 'bin/node-dev', '--unknown-arg', '--', 'test']);
242+
243+
t.same(nodeArgs, ['--unknown-arg']);
244+
t.end();
245+
});
246+
247+
tap.test('Single dash with value', t => {
248+
const { nodeArgs } = cli(['node', 'bin/node-dev', '-u', 'value', 'test']);
249+
250+
t.same(nodeArgs, ['-u=value']);
251+
t.end();
252+
});
253+
254+
tap.test('Single dash with = and value', t => {
255+
const { nodeArgs } = cli(['node', 'bin/node-dev', '-u=value', 'test']);
256+
257+
t.same(nodeArgs, ['-u=value']);
258+
t.end();
259+
});
260+
261+
tap.test('Single dash without value should fail', t => {
262+
t.throws(() => cli(['node', 'bin/node-dev', '-u', 'test']));
263+
t.end();
264+
});
265+
266+
tap.test('Single dash without value can use -- to delimit', t => {
267+
const { nodeArgs } = cli(['node', 'bin/node-dev', '-u', '--', 'test']);
268+
269+
t.same(nodeArgs, ['-u']);
270+
t.end();
271+
});
272+
273+
tap.test('Repeated single dash', t => {
274+
const { nodeArgs } = cli(['node', 'bin/node-dev', '-u=value1', '-u=value2', 'test']);
275+
276+
t.same(nodeArgs, ['-u=value1', '-u=value2']);
277+
t.end();
278+
});
279+
280+
tap.test('Repeated single dash without =', t => {
281+
const { nodeArgs } = cli(['node', 'bin/node-dev', '-u', 'value1', '-u', 'value2', 'test']);
282+
283+
t.same(nodeArgs, ['-u=value1', '-u=value2']);
284+
t.end();
285+
});

0 commit comments

Comments
 (0)