Skip to content

Conversation

@kdeldycke
Copy link
Collaborator

@kdeldycke kdeldycke commented Nov 11, 2025

@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 added bug f:context Feature for context object labels Nov 11, 2025
@kdeldycke kdeldycke linked an issue Nov 11, 2025 that may be closed by this pull request
@kdeldycke kdeldycke force-pushed the hide-sentinel-in-callbacks branch 2 times, most recently from b1ff8ed to 69f842d Compare November 11, 2025 09:47
@kdeldycke kdeldycke marked this pull request as ready for review November 11, 2025 09:48
@kdeldycke
Copy link
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
Copy link

mgorny commented Nov 11, 2025

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

@Rowlando13
Copy link
Collaborator

The tests look good.

@Rowlando13
Copy link
Collaborator

@davidism Can you review this?

@Rowlando13
Copy link
Collaborator

@kdeldycke Also add change entry.

@davidism
Copy link
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
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 kdeldycke force-pushed the hide-sentinel-in-callbacks branch from 69f842d to 437e1e3 Compare November 12, 2025 06:07
@kdeldycke
Copy link
Collaborator Author

@kdeldycke Also add change entry.

Ah yes, good call. Done.

@kdeldycke
Copy link
Collaborator Author

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

@kdeldycke
Copy link
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug f:context Feature for context object

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sentinel available in context during option callbacks

4 participants