Streamline option flag management - #3641
Merged
Rowlando13 merged 1 commit intoJul 16, 2026
Merged
Conversation
kdeldycke
force-pushed
the
option-flag-management-refactor
branch
2 times, most recently
from
June 27, 2026 08:20
3a9316a to
3b743b8
Compare
Member
|
@kdeldycke This looks good. @davidism Can you take a look? |
Collaborator
Author
|
Given #3676 , I guess the milestone change to 9.0.0 is just the blast radius of the recent |
kdeldycke
force-pushed
the
option-flag-management-refactor
branch
2 times, most recently
from
July 8, 2026 16:22
5f080f8 to
4e99c30
Compare
kdeldycke
force-pushed
the
option-flag-management-refactor
branch
from
July 11, 2026 14:01
4e99c30 to
8f30085
Compare
kdeldycke
added a commit
to kdeldycke/click-extra
that referenced
this pull request
Jul 16, 2026
Click's main branch (pallets/click#3641) stopped materializing an option's flag_value at construction: plain options and counters now carry the UNSET sentinel instead of None, and boolean flags carry it instead of True, the effective value moving to the new lazy flag_activation_value property. That sentinel leaked into two readers. option_value_kind() classified every plain option as taking an optional value (UNSET is not None), so generated Carapace specs marked required values with "?" instead of "=" and man pages dropped value metavars. And the --show-params table rendered the raw sentinel in its flag_value column instead of the value released Click materializes. Treat UNSET as "no declared flag value" in the classifier, and read flag_activation_value as the fallback in the table renderer, mirroring released Click's materialized values byte for byte on every supported version. Co-Authored-By: Claude <noreply@anthropic.com>
kdeldycke
added
parsing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This refactor is an attempt in streamlining the code around flag management in
Option.I hope this is not too big to digest. I did not went too far and just focused on
Optioninternals and flag-management. My goal is to colocate decision code by domain. Like having all prompt-related code in the same block. Or all type-resolution code together. Sometimes in the same code block, sometimes under a private utility function. My heuristic is mostly driven by the number of lines and the content of the docstring.Another idiom I introduced was using
@propertyforis_bool_flag. I didn't see that much propoerties used in Click, but these are easier to reason about for me.I also used the oportunity to introduce a type inference point in
_pick_typewhich is a small step echoing what @sirosen was proposing last year about having a separation between "data which is stored (where you might have UNSET) from the data which is viewed". That also resurface the infamousUNSETa bit, but hidden into a newflag_activation_valueproperty. And_hide_unsetis a way to delimit the internals state from the presetation state.This also contains some small syntax cleanups that were flagged by my linters.
Proof that this is the right time to introduce this refactor: after accumulating a lot of unitetsts over the past few years, I did not break any. And I did not discovered any new blind spot in the test suite.
I hope this will make code easier to read and maintain. But I need other eyes to check that.
My initial goal was way too ambitious as I want to create a
FlagOptionsubclass ofOption. But this is way too big and not sure I can pull it yet. So this is like a first step in that general direction.