Skip to content

Conversation

@kdeldycke
Copy link
Collaborator

This PR reduce the strong langage around the behavior of default=True aligning to flag_value for flag options. There is not need to declare that case legacy as it has been Click's behavior for a long time.

This address issues uncovered by @davidism in #3030 (comment) .

Context

This is a follow-up on #3030, which has been merged a little bit early and needs some polishing before Click 8.3.0 release.

Amends: 06847da

@kdeldycke
Copy link
Collaborator Author

@Rowlando13 As you are deep into the documentation side of Click, I'll be happy to have your feedback about the general wording here! :)

@kdeldycke kdeldycke force-pushed the adjust-doc-tone branch 2 times, most recently from 704fa64 to 7cfcd3f Compare August 25, 2025 06:32
@kdeldycke
Copy link
Collaborator Author

@Rowlando13 This is ready to be merged upstream! :)

@davidism
Copy link
Member

Good to merge, but I did mean to ask, is there a legitimate problem I'm not seeing here regarding typing/behavior? Or is it only a matter of being explicit with the code?

@kdeldycke
Copy link
Collaborator Author

Good to merge, but I did mean to ask, is there a legitimate problem I'm not seeing here regarding typing/behavior? Or is it only a matter of being explicit with the code?

Nothing about typing in this PR as most of the internals are already typed to t.Any.

Wait. Are you referring to typing as in text editing and not Python's type system? 😂

In which case I'm just requesting a quick review as I am not a native English speaker, so there's a non-zero chance my prose can end up a bit convoluted. 😬

@davidism
Copy link
Member

I just meant that in the original comments/discussion, you seemed to be saying there was an issue with the default. Is this something we should change in the future, or just a matter of consistency in type_value having the same type instead of True?

@kdeldycke
Copy link
Collaborator Author

I just meant that in the original comments/discussion, you seemed to be saying there was an issue with the default. Is this something we should change in the future, or just a matter of consistency in type_value having the same type instead of True?

You mean flag_value instead of type_value right?

In which case I was just warning that a flag cannot have its default value set to True, and be active by default (i.e. default=True) while at the same time having a flag_value set. Because it is that flag_value that will be used for default. That is more a matter of semantics as pointing to by @janluke in #1992.

Maybe we will refine the concepts in the future with a better separation of concerns but that's really far away. We have more important things to do. So no big deal.

@Rowlando13 Rowlando13 merged commit 10b77f9 into pallets:main Aug 28, 2025
10 checks passed
@kdeldycke
Copy link
Collaborator Author

Thanks @Rowlando13 for the merge!

@kdeldycke kdeldycke deleted the adjust-doc-tone branch August 28, 2025 05:47
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants