🐛 Ensure that an envvar set for a typer.Option list is split on whitespace#1791
Conversation
|
For what it's worth, this behavior is documented for click (which is where I got the idea from). Thanks again for your help @svlandeg! |
Yes, but the point is that it's not documented for Typer. And going forward after the vendoring, only what is documented by Typer is considered to be part of Typer's public API. So, if this is a feature we want to explicitly support and test, we should also document it. (But that's considered follow-up work & decisions as we have tried to be as minimally breaking for now) |
tiangolo
left a comment
There was a problem hiding this comment.
Sounds good to do this for now to keep the changes from the migration minimal, thank you! 🚀
This will be available in Typer 0.26.2 in the next few hours.
About deciding to keep it for the future or not, I'm undecided yet, as I suspect we'll refactor a bunch of this logic when we re-work how types work. For now, let's delay that decision for later.
Fixes #1789, thanks to @wpk-nist-gov for the report.
Description
This bug fix is related to #1787 though not quite the same. A user reported that this doesn't work anymore:
As far as I know, we don't actually document using
envvarin combination with alisttype. Before 0.26.0, Click's defaults would parse the envvar string on whitespace and produce a list when calling this with something likeenv={"ME": "rick morty"}.This currently still works for
Argumentas it doesn't have its own implementation ofvalue_from_envvar, it still uses Click's old one fromParameter. ForOption, this functionality was removed in the refactor. This PR restores the functionality forOptionin a quick bug fix PR.ArgumentandOptionshould work similarly, which is now the case with this PR. For the future, we should consider whether we want to keep & document this behaviour.AI Disclaimer
Used Cursor to help me debug the issue more quickly, manually reviewed everything in detail.
Checklist