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

feature: defaulted functionality #211

Merged
merged 4 commits into from Nov 1, 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
8 changes: 6 additions & 2 deletions README.md
Expand Up @@ -99,10 +99,14 @@ yargs engine.
* `argv`: an object representing the parsed value of `args`
* `key/value`: key value pairs for each argument and their aliases.
* `_`: an array representing the positional arguments.
* [optional] `--`: an array with arguments after the end-of-options flag `--`.
* `error`: populated with an error object if an exception occurred during parsing.
* `aliases`: the inferred list of aliases built by combining lists in `opts.alias`.
* `newAliases`: any new aliases added via camel-case expansion.
* `configuration`: the configuration loaded from the `yargs` stanza in package.json.
* `newAliases`: any new aliases added via camel-case expansion:
* `boolean`: `{ fooBar: true }`
* `defaulted`: any new argument created by `opts.default`, no aliases included.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juergba what's your reasoning for not including aliases? @coreyfarrell do you think you can work with this limitation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parse.detailed() also returns aliases which includes all aliases, so the information would be redundant.
It's no big effort to add the aliases to defaulted, I just need a decision.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my use it would be fine if defaulted did not list aliases, as long as setting an alias causes the original key to be excluded from defaulted. Would you mind adding a test for this?

    it('setting an alias removes associated key from defaulted', function () {
      var parsed = parser.detailed('--foo abc', {
        default: { kaa: 'abc' },
        alias: { foo: 'kaa' }
      })
      parsed.argv.should.deep.equal({ '_': [], kaa: 'abc', foo: 'abc' })
      parsed.defaulted.should.deep.equal({})
    })

Sorry I don't have a chance to actually run this test right now but in theory it should work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, I added your test.

* `boolean`: `{ foo: true }`
* `configuration`: given by default settings and `opts.configuration`.
<a name="configuration"></a>
Expand Down
7 changes: 5 additions & 2 deletions index.js
Expand Up @@ -35,6 +35,7 @@ function parse (args, opts) {
var notFlagsOption = configuration['populate--']
var notFlagsArgv = notFlagsOption ? '--' : '_'
var newAliases = {}
var defaulted = {}
// allow a i18n handler to be passed in, default to a fake one (util.format).
var __ = opts.__ || util.format
var error = null
Expand Down Expand Up @@ -317,7 +318,7 @@ function parse (args, opts) {
applyEnvVars(argv, false)
setConfig(argv)
setConfigObjects()
applyDefaultsAndAliases(argv, flags.aliases, defaults)
applyDefaultsAndAliases(argv, flags.aliases, defaults, true)
applyCoercions(argv)
if (configuration['set-placeholder-key']) setPlaceholderKeys(argv)

Expand Down Expand Up @@ -627,10 +628,11 @@ function parse (args, opts) {
return argv
}

function applyDefaultsAndAliases (obj, aliases, defaults) {
function applyDefaultsAndAliases (obj, aliases, defaults, canLog = false) {
Object.keys(defaults).forEach(function (key) {
if (!hasKey(obj, key.split('.'))) {
setKey(obj, key.split('.'), defaults[key])
if (canLog) defaulted[key] = true

;(aliases[key] || []).forEach(function (x) {
if (hasKey(obj, x.split('.'))) return
Expand Down Expand Up @@ -895,6 +897,7 @@ function parse (args, opts) {
error: error,
aliases: flags.aliases,
newAliases: newAliases,
defaulted: defaulted,
configuration: configuration
}
}
Expand Down
37 changes: 37 additions & 0 deletions test/yargs-parser.js
Expand Up @@ -1215,6 +1215,43 @@ describe('yargs-parser', function () {
parse.should.have.property('t', false).and.be.a('boolean')
parse.should.have.property('_').and.deep.equal(['moo'])
})

describe('track defaulted', function () {
it('should log defaulted options - not specified by user', function () {
var parsed = parser.detailed('', {
default: { foo: 'abc', 'bar.prop': 33, baz: 'x' },
configObjects: [{ baz: 'xyz' }]
})
parsed.argv.should.deep.equal({ '_': [], baz: 'xyz', foo: 'abc', bar: { prop: 33 } })
parsed.defaulted.should.deep.equal({ foo: true, 'bar.prop': true })
})

it('should not log defaulted options - specified without value', function () {
var parsed = parser.detailed('--foo --bar.prop', {
default: { foo: 'abc', 'bar.prop': 33 }
})
parsed.argv.should.deep.equal({ '_': [], foo: 'abc', bar: { prop: 33 } })
parsed.defaulted.should.deep.equal({})
})

it('should log defaulted options - no aliases included', function () {
var parsed = parser.detailed('', {
default: { kaa: 'abc' },
alias: { foo: 'kaa' }
})
parsed.argv.should.deep.equal({ '_': [], kaa: 'abc', foo: 'abc' })
parsed.defaulted.should.deep.equal({ kaa: true })
})

it('setting an alias excludes associated key from defaulted', function () {
var parsed = parser.detailed('--foo abc', {
default: { kaa: 'abc' },
alias: { foo: 'kaa' }
})
parsed.argv.should.deep.equal({ '_': [], kaa: 'abc', foo: 'abc' })
parsed.defaulted.should.deep.equal({})
})
})
})

describe('camelCase', function () {
Expand Down