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

soft-deprecate --reporter-options and replace with --reporter-option #3564

Closed
boneskull opened this issue Nov 11, 2018 · 5 comments
Closed
Labels
area: node.js command-line-or-Node.js-specific

Comments

@boneskull
Copy link
Contributor

This assumes #3556 lands. This issue came out of code review there.

Yargs has great support for "array"-type options. Generally, any option can be specified multiple times. In addition, options can accept multiple, space-delimited values, e.g., --foo bar baz quux.

This issue has two parts:

  1. Soft-deprecate (using utils.deprecate()) --reporter-options
  2. Implement --reporter-option which can be repeated multiple times.

It'd be cool to be able to specify a minimum of one value and a maximum of two, e.g.:

--reporter-option someBooleanFlag --reporter-option someString stringValue

Not sure if Yargs can to that, but if it can't, then require a single value and split the result on =:

--reporter-option someBooleanFlag --reporter-option someString=stringValue
@boneskull boneskull added status: accepting prs Mocha can use your help with this one! area: usability concerning user experience or interface semver-minor implementation requires increase of "minor" version number; "features" area: node.js command-line-or-Node.js-specific labels Nov 11, 2018
@plroebuck
Copy link
Contributor

See discussion from #3487. @johanblumenberg provided the PR's implementation.

Would strongly prefer that reporter name be prefixed to key in --reporter-option key=value.
For example, if specifying "xunit" reporter output file, then:
--reporter-option xunit.output=mocha-xunit.xml

We should decide how to address these potential problems first.

@boneskull
Copy link
Contributor Author

I'm closing this; I've since gotten some backchannel feedback that comma-sep is useful, and it's really not much trouble to support both styles. as of v6, --reporter-option and --reporter-options are the same thing, and can be described using commas (--reporter-option foo=bar,baz=quux) or repetition (--reporter-option foo=bar --reporter-option baz=quux)

@plroebuck plroebuck reopened this Jan 30, 2019
@plroebuck
Copy link
Contributor

plroebuck commented Jan 30, 2019

Wait. I don't care whether --reporterOptions is deprecated or not.

But need --reporterOption to be able to process comma-separated arguments as the value
(e.g., --reporterOption meta=foo,bar,baz) which cannot currently be done if using exact same code.

@boneskull
Copy link
Contributor Author

mm, I think all it needs is a coerce: list in lib/cli/run.js.

@boneskull boneskull added type: bug a defect, confirmed by a maintainer and removed status: accepting prs Mocha can use your help with this one! semver-minor implementation requires increase of "minor" version number; "features" area: usability concerning user experience or interface type: bug a defect, confirmed by a maintainer labels Jan 31, 2019
@boneskull
Copy link
Contributor Author

let's keep this closed and just make a new issue, nevermind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: node.js command-line-or-Node.js-specific
Projects
None yet
Development

No branches or pull requests

2 participants