Preserve groups containing global keys #395
Conversation
| @@ -385,7 +396,6 @@ function Argv (processArgs, cwd) { | |||
| return options | |||
| } | |||
|
|
|||
| var groups = {} | |||
There was a problem hiding this comment.
FYI reset() seems to be called before this is reached. groups should be declared higher up regardless of this PR.
|
@novemberborn Hey thanks! I think this is certainly a reasonable request - the groups for global options should be preserved for commands just like the options themselves are. Since the concept of global options is so new to yargs, I think we just haven't shaken out all the proper uses/bugs for them. I think this looks good. My only concern is that, per #336 and the failing test, yargs should output the command-specific option groups before it outputs the global option groups, so some reordering seems necessary. Could you possibly work that into this PR and also add a specific test for preserving groups with global options (one that actually uses |
novemberborn
changed the title
novemberborn
force-pushed
the
retain-global-groups
branch
from
e52afb5
to
f7f9709
Compare
|
@nexdrew cool, thanks! I've (force-)pushed a commit which addresses your concerns. By default the preserved groups are shown last, but you can add to such a group or indeed bump it up in the command builder. |
| return yargs(['upload', '-h']) | ||
| .command('upload', 'upload something', function (yargs) { | ||
| return yargs | ||
| .group([], 'Awesome Flags:') |
There was a problem hiding this comment.
This is an interesting "feature". 😄 I like the functionality, but the API is not very intuitive IMO. That being said, I can't really think of a better alternative, so 👍.
There was a problem hiding this comment.
Heh yea. It's a side-effect of being able to add to preserved groups but I figured it'd be worthwhile adding a test for it.
There was a problem hiding this comment.
Without the test, I would not have spotted this. So great idea!
|
LGTM 👍 @novemberborn Thanks for the contribution! |
|
@bcoe Any objections here? |
|
@novemberborn @nexdrew I love it, will work on getting a release out tomorrow, and on moving yargs to an org. |
Preserve groups containing global keys
|
@novemberborn give |
|
@bcoe works great! |
Heya! I used yargs in a new CLI tool the other day. This tool takes some global options as well as per-command options. I noticed I had to restore the global option groups for each command, which I was not expecting.
With this PR I'm proposing that groups containing global keys are preserved within commands. Please let me know if that's a sensible thing for yargs to do!
One concern raised in #336 is the ability to control the order of the groups in each command's help output. Currently this PR puts the groups with global keys first. Possibly the
groupsobject could be reordered if the group is redeclared without keys.I also may be misunderstanding the myriad uses of
yargs.reset(), though the tests seem to pass aside for one introduced to fix #336. I can fix those tests if people are happy with this approach.Thanks.