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 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
26 changes: 26 additions & 0 deletions test/yargs-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -1215,6 +1215,32 @@ describe('yargs-parser', function () {
parse.should.have.property('t', false).and.be.a('boolean')
parse.should.have.property('_').and.deep.equal(['moo'])
})

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 })
})
})

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