Skip to content

Commit

Permalink
fix: support multiple values after --ignore-files again (#1652)
Browse files Browse the repository at this point in the history
The yargs upgrade from 6.6.0 to 13.2.1 in edebef4 broke the
documented ability to pass multiple files to ignore after the
`--ignore-files` (aka `-i`) parameter.

This is because yargs 11 started to treat `requiresArg` as an implicit
`nargs: 1`, i.e. to accept exactly one value only.

This patch fixes the issue by dropping `requiresArg` and validating the
number of parameters in the `execute` function instead.
  • Loading branch information
Rob--W authored and rpl committed Jul 12, 2019
1 parent 626a180 commit ff0ffa5
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
11 changes: 10 additions & 1 deletion src/program.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ export class Program {

const argv = this.yargs.argv;

// Replacement for the "requiresArg: true" parameter until the following bug
// is fixed: https://github.com/yargs/yargs/issues/1098
if (argv.ignoreFiles && !argv.ignoreFiles.length) {
throw new UsageError('Not enough arguments following: ignore-files');
}

const cmd = argv._[0];

const version = getVersion(this.absolutePackageDir);
Expand Down Expand Up @@ -348,7 +354,10 @@ Example: $0 --help run.
'ignored. (Example: --ignore-files=path/to/first.js ' +
'path/to/second.js "**/*.log")',
demandOption: false,
requiresArg: true,
// The following option prevents yargs>=11 from parsing multiple values,
// so the minimum value requirement is enforced in execute instead.
// Upstream bug: https://github.com/yargs/yargs/issues/1098
// requiresArg: true,
type: 'array',
},
'no-input': {
Expand Down
24 changes: 24 additions & 0 deletions tests/unit/test.program.js
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,30 @@ describe('program.main', () => {

sinon.assert.called(logStream.makeVerbose);
});

it('requires a parameter after --ignore-files', async () => {
const fakeCommands = fake(commands);
return execProgram(['build', '--ignore-files'], {commands: fakeCommands})
.then(makeSureItFails())
.catch((error) => {
assert.match(
error.message, /Not enough arguments following: ignore-files/);
});
});

it('supports multiple parameters after --ignore-files', async () => {
const fakeCommands = fake(commands, {
build: () => Promise.resolve(),
});
return execProgram(
['build', '--ignore-files', 'f1', 'f2', '-a', 'xxx', '-i', 'f4', 'f3'],
{commands: fakeCommands})
.then(() => {
const options = fakeCommands.build.firstCall.args[0];
assert.deepEqual(options.ignoreFiles, ['f1', 'f2', 'f4', 'f3']);
assert.equal(options.artifactsDir, 'xxx');
});
});
});

describe('program.defaultVersionGetter', () => {
Expand Down

0 comments on commit ff0ffa5

Please sign in to comment.