Skip to content

Provide altered context to callbacks to hide UNSET values as None - #3137

Merged
Rowlando13 merged 1 commit into
pallets:stablefrom
kdeldycke:hide-sentinel-in-callbacks
Nov 12, 2025
Merged

Rowlando13 merged 1 commit into
pallets:stablefrom
kdeldycke:hide-sentinel-in-callbacks

Conversation

@kdeldycke

@kdeldycke kdeldycke commented Nov 11, 2025 •

Copy link
Copy Markdown
Collaborator

@kdeldycke kdeldycke self-assigned this Nov 11, 2025
@kdeldycke
kdeldycke marked this pull request as draft November 11, 2025 07:59
@kdeldycke kdeldycke added this to the 8.3.1 milestone Nov 11, 2025
@kdeldycke kdeldycke linked an issue Nov 11, 2025 that may be closed by this pull request
@kdeldycke
kdeldycke marked this pull request as ready for review November 11, 2025 09:48
@kdeldycke

Copy link
Copy Markdown
Collaborator Author

Ok I found a solution to reconcile the 2 issues. It's a bit dirty, but it works without side-effects.

@kdeldycke kdeldycke changed the title WIP: Add test to highlight sentinels being passed in callbacks Provide altered context to callbacks to hide UNSET values as None Nov 11, 2025
@mgorny

mgorny commented Nov 11, 2025

Copy link
Copy Markdown

Thanks! I can confirm that this fixes the issues I've seen in black.

@Rowlando13

Copy link
Copy Markdown
Member

The tests look good.

@Rowlando13

Copy link
Copy Markdown
Member

@davidism Can you review this?

@Rowlando13

Copy link
Copy Markdown
Member

@kdeldycke Also add change entry.

@davidism

Copy link
Copy Markdown
Member

A ridiculous fix 😅, but I'm fine with it.

Maybe in the next release we can figure out some internal API separate from the external one, rather than this bit of magic.

@kdeldycke

Copy link
Copy Markdown
Collaborator Author

A ridiculous fix 😅, but I'm fine with it.

Oh yes. A bit ashamed by that. That's why I tried to be very explicit in delimiting that fix. Anyone reaching that implementation will understand that this part needs some refactor.

Maybe in the next release we can figure out some internal API separate from the external one, rather than this bit of magic.

Oh yes. You initiated that direction by helping me identify private methods in the previous months, with :meta private: tags in docstrings. Maybe we can go further and start prefixing functions, methods and properties with underscore _, as a hint for stuff you are not supposed to mess with. Let's discuss that once things settle down.

@kdeldycke

Copy link
Copy Markdown
Collaborator Author

@kdeldycke Also add change entry.

Ah yes, good call. Done.

@kdeldycke

Copy link
Copy Markdown
Collaborator Author

Also, just for reference, this PR pass all my tricky hacks in Click Extra. 😇

@kdeldycke

Copy link
Copy Markdown
Collaborator Author

Maybe in the next release we can figure out some internal API separate from the external one, rather than this bit of magic.

Oh yes. You initiated that direction by helping me identify private methods in the previous months, with :meta private: tags in docstrings. Maybe we can go further and start prefixing functions, methods and properties with underscore _, as a hint for stuff you are not supposed to mess with. Let's discuss that once things settle down.

Also, the advantage of all that UNSET saga is that we can now easily find where in the code base of Click there is expectation of value normalization. Just grep on UNSET! 😅🫣

@Rowlando13
Rowlando13 merged commit b91bb95 into pallets:stable Nov 12, 2025
12 checks passed
@kdeldycke
kdeldycke deleted the hide-sentinel-in-callbacks branch November 16, 2025 05:34
@pallets pallets deleted a comment from ayushtonystar-del Nov 16, 2025
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Dec 1, 2025
@kdeldycke kdeldycke added parsing Parsing, parameters, commands, chaining, context and removed f:context labels Aug 8, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

parsing Parsing, parameters, commands, chaining, context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sentinel available in context during option callbacks

4 participants