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

fix: positionals should not overwrite options #1992

Merged
merged 5 commits into from Aug 4, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 12 additions & 1 deletion lib/command.ts
Expand Up @@ -598,7 +598,18 @@ export class CommandInstance {
// any new aliases need to be placed in positionalMap, which
// is used for validation.
if (!positionalMap[key]) positionalMap[key] = parsed.argv[key];
argv[key] = parsed.argv[key];
// Addresses: https://github.com/yargs/yargs/issues/1637
// If both positionals/options provided,
// and if at least one is an array: don't overwrite, combine.
if (
Object.prototype.hasOwnProperty.call(argv, key) &&
Object.prototype.hasOwnProperty.call(parsed.argv, key) &&
(Array.isArray(argv[key]) || Array.isArray(parsed.argv[key]))
) {
argv[key] = ([] as string[]).concat(argv[key], parsed.argv[key]);
} else {
argv[key] = parsed.argv[key];
}
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion lib/yargs-factory.ts
Expand Up @@ -809,7 +809,7 @@ export class YargsInstance {
);
} else {
globals.forEach(g => {
if (this.#options.local.indexOf(g) === -1) this.#options.local.push(g);
if (!this.#options.local.includes(g)) this.#options.local.push(g);
});
}
return this;
Expand Down
52 changes: 52 additions & 0 deletions test/command.cjs
Expand Up @@ -209,6 +209,58 @@ describe('Command', () => {
argv['--'].should.eql(['apple', 'banana']);
called.should.equal(true);
});

// Addresses: https://github.com/yargs/yargs/issues/1637
it('does not overwrite options in argv if variadic', () => {
yargs
.command({
command: 'cmd [foods..]',
desc: 'cmd desc',
builder: yargs =>
yargs.positional('foods', {
desc: 'foods desc',
type: 'string',
}),
handler: argv => {
argv.foods.should.deep.equal(['apples', 'cherries', 'grapes']);
},
})
.parse('cmd --foods apples cherries grapes');
});

it('does not overwrite options in argv if variadic and when using default command', () => {
yargs
.command({
command: '$0 [foods..]',
desc: 'default desc',
builder: yargs =>
yargs.positional('foods', {
desc: 'foods desc',
type: 'string',
}),
handler: argv => {
argv.foods.should.deep.equal(['apples', 'cherries', 'grapes']);
},
})
.parse('--foods apples cherries grapes');
});

it('does not overwrite options in argv if variadic and preserves falsey values', () => {
yargs
.command({
command: '$0 [numbers..]',
desc: 'default desc',
builder: yargs =>
yargs.positional('numbers', {
desc: 'numbers desc',
type: 'number',
}),
handler: argv => {
argv.numbers.should.deep.equal([0, 1, 2]);
},
})
.parse('--numbers 0 1 2');
});
});

describe('variadic', () => {
Expand Down