Prevent empty default value in CLI options #3441
Conversation
If empty array is default value([]), it displayed as `default: ` Signed-off-by: Outsider <outsideris@gmail.com>
|
Certainly understand having fix on our side. But why was there no corresponding issue created for "tj/commander"? Would prefer to leave maintenance of their bugs to them. |
|
@plroebuck As I said that in Alternate Designs, I'm not sure it's a bug in commander.js because it's related with how to use default vault and we can fix it on our side easily. |
|
Think it's their problem - opened issue. |
| @@ -763,9 +763,9 @@ Mocha supports the `err.expected` and `err.actual` properties of any thrown `Ass | |||
| -w, --watch watch files for changes | |||
| --check-leaks check for global variable leaks | |||
| --full-trace display the full stack trace | |||
| --compilers <ext>:<module>,... use the given module(s) to compile files (default: ) | |||
| --compilers <ext>:<module>,... use the given module(s) to compile files | |||
There was a problem hiding this comment.
So for all of these README changes, why did you remove the default text altogether rather than just adding [] where it was missing?
|
Tangentially related only. Why not? .option('-s, --slow <ms>', '"slow" test threshold in milliseconds', Number, 75)
.option('-t, --timeout <ms>', 'set test-case timeout in milliseconds', Number, 2000) |
|
I see. I saw the opened PR in commander.js. |
|
Uh, why'd you close this? The README is still messed up, and we'll still need to make changes to "package.json" and "package-lock.json" to fix even after their fix is available. |
|
Change title from:
to
|
outsideris
changed the title
|
@plroebuck As I leave comment in #3433 , we can open new PR after commander.js fixed it. |
Description of the Change
If empty array is default value([]), it displayed as
default:With
commanderand a default value is an empty array([]), it shows like this:(default: )is confusing to users.So, this PR doesn't use a default value with an empty array. Instead, it set an empty array manually.
Alternate Designs
Let it as
(default: )or fix it in commander.js.Possible Drawbacks
I'm not sure.
Applicable issues
Fix #3433