-
-
Notifications
You must be signed in to change notification settings - Fork 699
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
✨ Implement list parsing from string with separators. #800
base: master
Are you sure you want to change the base?
✨ Implement list parsing from string with separators. #800
Conversation
📝 Docs preview for commit 7eb3fd2 at: https://d9b28bcb.typertiangolo.pages.dev |
📝 Docs preview for commit cec52c9 at: https://0ff1aa3c.typertiangolo.pages.dev |
Hi, thanks for your PR! I'll put this in draft as long as the test suite is failing. |
📝 Docs preview for commit aca33a5 at: https://d79559eb.typertiangolo.pages.dev |
📝 Docs preview for commit 46b4690 at: https://3c495073.typertiangolo.pages.dev |
📝 Docs preview for commit c0aba28 at: https://10533715.typertiangolo.pages.dev |
📝 Docs preview for commit 6233289 at: https://722bbed7.typertiangolo.pages.dev |
📝 Docs preview for commit f09da73 at: https://39b00efc.typertiangolo.pages.dev |
📝 Docs preview for commit 3cdd682 at: https://809b191b.typertiangolo.pages.dev |
📝 Docs preview for commit 03558cb at: https://9744c225.typertiangolo.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks 💚
📝 Docs preview for commit e54193b at: https://4d37e64b.typertiangolo.pages.dev |
📝 Docs preview for commit 1793f0f at: https://919ea834.typertiangolo.pages.dev |
📝 Docs preview for commit ae11002 at: https://411417d0.typertiangolo.pages.dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for the PR! It's really well written, with the appropriate changes and additions to the documentation, example code and unit tests. Really appreciated 🙏
I pushed a few smaller edits to update the documentation as well as pointing out that it does in fact work without quotes, as long as there is no whitespace inbetween, e.g.
python main.py --number 2,3,4.5
The PR looks good to merge to me, but I defer to Tiangolo for the final decision.
📝 Docs preview for commit de078a6 at: https://e3977423.typertiangolo.pages.dev Modified Pages |
Is there anything I can contribute? |
At this point the PR is in a good shape to be reviewed by Tiangolo - we've got this on our internal board for him to review when he has time 😉 Thanks for your patience! |
📝 Docs preview for commit 92a6d81 at: https://b1168530.typertiangolo.pages.dev Modified Pages |
92a6d81
to
d5328fa
Compare
📝 Docs preview for commit d5328fa at: https://2b5a4b04.typertiangolo.pages.dev Modified Pages |
This would be great! Much more convenient to pass a large number of elements to an option 💯 |
Implements feature #554.
This merge request introduces the functionality to parse List[T] options from strings using a specified separator. An example of how this can be utilized is demonstrated below:
The implementation works by overwriting the
process_value
function of the click parser, doing string splitting before forwarding the result to click. This ensures full compatibility with the existing features of both Click and Typer. The feature does not allow whitespace only separators to avoid ambiguities.Feedback is much appreciated!
Open questions:
typer.Option
, which delays error detection until the command execution. Should we consider moving this error handling to theOptionInfo
constructor to allow for earlier failure detection?ToDo: