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: -- (stop parse) was not being respected by commands #1459

Merged
merged 2 commits into from Oct 30, 2019
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
12 changes: 10 additions & 2 deletions lib/command.js
Expand Up @@ -227,9 +227,12 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {

if (commandHandler.handler && !yargs._hasOutput()) {
yargs._setHasOutput()
// to simplify the parsing of positionals in commands,
// we temporarily populate '--' rather than _, with arguments
const populateDoubleDash = !!yargs.getOptions().configuration['populate--']
if (!populateDoubleDash) yargs._copyDoubleDash(innerArgv)

innerArgv = applyMiddleware(innerArgv, yargs, middlewares, false)

let handlerResult
if (isPromise(innerArgv)) {
handlerResult = innerArgv.then(argv => commandHandler.handler(argv))
Expand Down Expand Up @@ -349,7 +352,12 @@ module.exports = function command (yargs, usage, validation, globalMiddleware) {
// short-circuit parse.
if (!unparsed.length) return

const parsed = Parser.detailed(unparsed, options)
const config = Object.assign({}, options.configuration, {
'populate--': true
})
const parsed = Parser.detailed(unparsed, Object.assign({}, options, {
configuration: config
}))

if (parsed.error) {
yargs.getUsageInstance().fail(parsed.error.message, parsed.error)
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -49,7 +49,7 @@
},
"scripts": {
"fix": "standard --fix",
"pretest": "standard",
"posttest": "standard",
"test": "c8 --reporter=html --reporter=text mocha --require ./test/before.js --timeout=12000 --check-leaks",
"coverage": "c8 report --reporter=text-lcov | coveralls",
"release": "standard-version"
Expand Down
39 changes: 39 additions & 0 deletions test/command.js
Expand Up @@ -135,6 +135,45 @@ describe('Command', () => {
.parse()
parseCount.should.equal(1)
})

// see: https://github.com/yargs/yargs/issues/1457
it('handles -- in conjunction with positional arguments', () => {
let called = false
const argv = yargs('foo hello world series -- apple banana')
.command('foo <bar> [awesome...]', 'my awesome command', noop, (argv2) => {
argv2.bar.should.eql('hello')
argv2.awesome.should.eql(['world', 'series'])
argv2['_'].should.eql(['foo', 'apple', 'banana'])
called = true
})
.parse()
argv.bar.should.eql('hello')
argv.awesome.should.eql(['world', 'series'])
argv['_'].should.eql(['foo', 'apple', 'banana'])
called.should.equal(true)
})

// see: https://github.com/yargs/yargs/issues/1457
it('continues to support populate-- for commands, post #1457', () => {
let called = false
const argv = yargs('foo hello world series -- apple banana')
.command('foo <bar> [awesome...]', 'my awesome command', noop, (argv2) => {
argv2.bar.should.eql('hello')
argv2.awesome.should.eql(['world', 'series'])
argv2['_'].should.eql(['foo'])
argv2['--'].should.eql(['apple', 'banana'])
called = true
})
.parserConfiguration({
'populate--': true
})
.parse()
argv.bar.should.eql('hello')
argv.awesome.should.eql(['world', 'series'])
argv['_'].should.eql(['foo'])
argv['--'].should.eql(['apple', 'banana'])
called.should.equal(true)
})
})

describe('variadic', () => {
Expand Down
36 changes: 28 additions & 8 deletions yargs.js
Expand Up @@ -1030,14 +1030,21 @@ function Yargs (processArgs, cwd, parentRequire) {
enumerable: true
})

self._parseArgs = function parseArgs (args, shortCircuit, _skipValidation, commandIndex) {
let skipValidation = !!_skipValidation
self._parseArgs = function parseArgs (args, shortCircuit, _calledFromCommand, commandIndex) {
let skipValidation = !!_calledFromCommand
args = args || processArgs

options.__ = y18n.__
options.configuration = self.getParserConfiguration()

const parsed = Parser.detailed(args, options)
const populateDoubleDash = !!options.configuration['populate--']
const config = Object.assign({}, options.configuration, {
'populate--': true
})
const parsed = Parser.detailed(args, Object.assign({}, options, {
configuration: config
}))

let argv = parsed.argv
if (parseContext) argv = Object.assign({}, argv, parseContext)
const aliases = parsed.aliases
Expand All @@ -1052,7 +1059,7 @@ function Yargs (processArgs, cwd, parentRequire) {
// are two passes through the parser. If completion
// is being performed short-circuit on the first pass.
if (shortCircuit) {
return argv
return (populateDoubleDash || _calledFromCommand) ? argv : self._copyDoubleDash(argv)
}

// if there's a handler associated with a
Expand Down Expand Up @@ -1085,7 +1092,8 @@ function Yargs (processArgs, cwd, parentRequire) {
// commands are executed using a recursive algorithm that executes
// the deepest command first; we keep track of the position in the
// argv._ array that is currently being executed.
return command.runCommand(cmd, self, parsed, i + 1)
const innerArgv = command.runCommand(cmd, self, parsed, i + 1)
return populateDoubleDash ? innerArgv : self._copyDoubleDash(innerArgv)
} else if (!firstUnknownCommand && cmd !== completionCommand) {
firstUnknownCommand = cmd
break
Expand All @@ -1094,7 +1102,8 @@ function Yargs (processArgs, cwd, parentRequire) {

// run the default command, if defined
if (command.hasDefaultCommand() && !skipDefaultCommand) {
return command.runCommand(null, self, parsed)
const innerArgv = command.runCommand(null, self, parsed)
return populateDoubleDash ? innerArgv : self._copyDoubleDash(innerArgv)
}

// recommend a command if recommendCommands() has
Expand All @@ -1111,7 +1120,8 @@ function Yargs (processArgs, cwd, parentRequire) {
self.exit(0)
}
} else if (command.hasDefaultCommand() && !skipDefaultCommand) {
return command.runCommand(null, self, parsed)
const innerArgv = command.runCommand(null, self, parsed)
return populateDoubleDash ? innerArgv : self._copyDoubleDash(innerArgv)
}

// we must run completions first, a user might
Expand All @@ -1129,7 +1139,7 @@ function Yargs (processArgs, cwd, parentRequire) {

self.exit(0)
})
return argv
return (populateDoubleDash || _calledFromCommand) ? argv : self._copyDoubleDash(argv)
}

// Handle 'help' and 'version' options
Expand Down Expand Up @@ -1173,6 +1183,16 @@ function Yargs (processArgs, cwd, parentRequire) {
else throw err
}

return (populateDoubleDash || _calledFromCommand) ? argv : self._copyDoubleDash(argv)
}

// to simplify the parsing of positionals in commands,
// we temporarily populate '--' rather than _, with arguments
// after the '--' directive. After the parse, we copy these back.
self._copyDoubleDash = function (argv) {
if (!argv._) return argv
argv._.push.apply(argv._, argv['--'])
delete argv['--']
return argv
}

Expand Down