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

Choice Options With Groups not grouping the options in the help message #88

Closed
rkklai opened this issue Sep 12, 2019 · 3 comments · Fixed by #89
Closed

Choice Options With Groups not grouping the options in the help message #88

rkklai opened this issue Sep 12, 2019 · 3 comments · Fixed by #89

Comments

@rkklai
Copy link

rkklai commented Sep 12, 2019

Currently the ChoiceGroup.provideDelegate doesn't register the option groups. So the sub group options are not properly grouped.

override fun provideDelegate(thisRef: CliktCommand, prop: KProperty<*>): ReadOnlyProperty<CliktCommand, OutT> {
        option.provideDelegate(thisRef, prop) // infer the option name and register it
        thisRef.registerOptionGroup(this)
        for ((_, group) in groups) {
>>>>> thisRef.registerOptionGroup(group) <<<<<< need to register the sub groups here?
            for (option in group.options) {
                option.parameterGroup = this
                option.groupName = group.groupName
                thisRef.registerOption(option)
            }
        }
        return this
    }
@ajalt
Copy link
Owner

ajalt commented Sep 12, 2019

Thanks for the report. I'm not sure I understand the issue. Can you share a minimal program that demonstrates the problem?

@rkklai
Copy link
Author

rkklai commented Sep 12, 2019

I think I found the bug. It's actually in the Option interface. The ChoiceGroup added the option with the ChoiceGroup as the parmeterGroup whose groupName is null, and the option.groupName is set to the ChoiceGroup.groups[i].name. I think the groupName init code should have "?: groupName" to fall back to the option's own groupName even if it's parameterGroup?.groupName is null.

The example at https://ajalt.github.io/clikt/options/#choice-options-with-groups shows this problem as the help output are not grouped according to the two options: "Options for loading from disk" and "Option for loading from network".

P.S. Thanks for this amazing project!

 /** Information about this option for the help output. */
    val parameterHelp: HelpFormatter.ParameterHelp.Option?
        get() = when {
            hidden -> null
            else -> HelpFormatter.ParameterHelp.Option(names, secondaryNames, metavar, help, nvalues, helpTags,
                    groupName = if (this is GroupableOption) parameterGroup?.groupName ?: groupName else null)
        }

@ajalt
Copy link
Owner

ajalt commented Sep 13, 2019

You're completely correct. Thanks for looking into it!

@ajalt ajalt closed this as completed in #89 Sep 13, 2019
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 a pull request may close this issue.

2 participants