options inherited from context settings aren't applied

[seanbreckenridge]

Thanks for the library, love that its a drop in replacement

Just creating this to track this here/let you know.

Defaults set in context settings aren't detected by command/groups

Minimal example:

import os

USE_RICH = "USE_RICH" in os.environ

if USE_RICH:
    import rich_click as click  # type: ignore[import]
else:
    import click  # type: ignore[no-redef]

DEFAULT_CONTEXT_SETTINGS = {"show_default": True}


@click.command(context_settings=DEFAULT_CONTEXT_SETTINGS)
@click.option("--use-context-settings", default="should work", help="help text shows")
@click.option("--overwrite-to-succeed", default="does work", show_default=True, help="shows here too")
def main(use_context_settings, overwrite_to_succeed) -> None:
    click.echo(use_context_settings)
    click.echo(overwrite_to_succeed)

if __name__ == "__main__":
    main()

Would be willing to create a PR for this at some point in the future, I would imagine one just has to look these up in the parent context instead of just checking the parameters passed, but may be a bit more complicated than that...

[ewels]

Ah well spotted! Many thanks for the minimal example too.. A PR would be great if you get a chance (I'm away on holiday currently) otherwise I'll try to look into it when I get a chance 👍🏻

[seanbreckenridge]

Tried creating a basic helper, but hit the wall I was expecting, not sure how click handles this

In particular this stuff:

    show_default_val = _get_param_value(param, ctx, "show_default")
    ## hmm -- show_default returns False even when ctx has a value set, since
    # show_default=False is the default for the parameter? Not sure how click
    # properly resolves this

[OriBenHur-akeyless]

Are there any plans to fix this bug?

[ewels]

Yes - it was hopefully fixed in #89 but that PR went quiet. I recently ported it to #103 where I'm trying to resolve merge conflicts (#104). Once those are both merged, hopefully this issue should be resolved.

I've been slow to merge because #89 is a massive PR that affects a lot of the package.

[ewels]

Hopefully resolved by merging #89 - please let me know this is not the case.

[OriBenHur-akeyless]

The defaults are still not being shown

[OriBenHur-akeyless]

any updates on that?

[ewels]

I've had the browser tab for this open for over a month but no, not had a chance to look at it yet sorry. PRs welcome.

[dwreeves]

Hey, I have a fix for this! 😄

But before I implement it, I think we'll need to expand the test coverage to different versions of Click. Long story short, Click 7, 8.0, and 8.1 all behave slightly differently.

I will work on doing that first, then we can merge in the fix.

[kdeldycke]

The whole inheritance of Defaults / Environment variables / Configuration file / CLI parameters and their precedence is quite complex in Click. It doesn't indeed work out of the box and that's not rich-click fault! 😅

That being said, I think I also solved this issue in Click Extra, so feel free to have a look if your looking for inspiration or ideas: https://github.com/kdeldycke/click-extra

[OriBenHur-akeyless]

Any news?

[dwreeves]

I need the test suite changes in to solve this since there are different behaviors in different Click versions. But once that's in I can merge a fix.

[ewels]

Apologies all, I'm struggling to find time to dedicate to the maintenance of this project - it's become a lot more popular than I anticipated and passed the "does what I need it to do" threshold a long time ago. I was kind of hoping to have the functionality wrapped into another library by now 😅 (as it has been for Typer, in fairness). I'd be very happy to expand the team of maintainers (add collaborators to the repo who can review + merge) if anyone is interested, or any other solutions.

Anyway, I'll merge the testing PR now @dwreeves (thanks for this work!). But please note that the merges are getting more and more blind on my part..

[dwreeves]

Thanks for your work with this library @ewels, and totally understood that maintaining things for a long time can be quite the commitment. Part of why I added the test suite changes were to help provide some insight into what code changes were doing across all possible users, which can reduce maintenance burdens.

I'll get working on addressing this issue today; I'm going to be doing a little café personal work sesh in an hour.

[dwreeves]

Pull request resolving this issue has been opened: #115

[dwreeves]

Thank you again for opening this issue, and for everyone else who asked for a fix. We have gotten around to fixing it. We'll be shipping this fix and some more fixes in an upcoming release.