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 option source value #1788

Closed
wants to merge 2 commits into from
Closed

fix option source value #1788

wants to merge 2 commits into from

Conversation

mshima
Copy link
Contributor

@mshima mshima commented Aug 31, 2022

Pull Request

Problem

'config' option source looks incorrect:

this.setOptionValueWithSource(impliedKey, option.implied[impliedKey], 'implied');

Solution

ChangeLog

@mshima
Copy link
Contributor Author

mshima commented Aug 31, 2022

maybe implied is missing, instead?

@shadowspawn
Copy link
Collaborator

Yes.

config appears in code as available for author use, although not set by Commander. #1584 #1613

implied is set by Commander, but wasn't added to the JSDoc or TSDoc listing the known values. #1724

@mshima
Copy link
Contributor Author

mshima commented Aug 31, 2022

Is there any example on how to use setOptionValueWithSource with 'config'?
Something like this?

const options = program.option('--config', 'config.js', 'config.js').parse().opts();
if (options.config) {
  const config = require(options.config);
  for (const [opt, value] of Object.entries(config)) {
    program.setValueWithSource(opt, value, 'config');
  }
}
const updatedOptions = program.opts();

@shadowspawn
Copy link
Collaborator

The use case I had in mind was one of the scenarios in #1584

  1. configure program (which includes defaults)
  2. read and store config
  3. program.parse()

@mshima
Copy link
Contributor Author

mshima commented Sep 1, 2022

The use case I had in mind was one of the scenarios in #1584

  1. configure program (which includes defaults)
  2. read and store config
  3. program.parse()

The problem of this workflow is that custom config needs to be parsed manually.
Maybe an event like on('options:parsed', loadConfigurationFiles).

@mshima mshima deleted the patch-1 branch September 1, 2022 12:08
@shadowspawn
Copy link
Collaborator

A big disadvantage of events is the lack of support for async. The life cycle hooks could be better for something which might include async file handling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants