-
-
Notifications
You must be signed in to change notification settings - Fork 772
✨ Add module:function
syntax support for typer CLI
#1275
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
base: master
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Not sure why CI failed, maybe it needs a label added (triage)? Let me know if I can assist 👍 |
module:function
syntax support for typer CLImodule:function
syntax support for typer CLI
2137408
to
88b4ffb
Compare
Rebased to update with trunk, all checks passed 😄 |
📝 Docs preview for commit 88b4ffb at: https://d829fb58.typertiangolo.pages.dev Modified Pages |
Great, this looks ready @svlandeg 😄 |
Thanks @lmmx! I'll try and review this soonish. |
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.
Wouldn't this break the use-case where Windows users specify a full path with C:\somepath
?
Oh great catch ! Yes we can check that, very good import os
def maybe_update_state(ctx: click.Context) -> None:
path_or_module = ctx.params.get("path_or_module")
if path_or_module and ":" in path_or_module:
drive, _ = os.path.splitdrive(path_or_module)
if not drive: # avoid misinterpreting "C:\..." on Windows
module_part, func_part = path_or_module.rsplit(":", 1)
path_or_module = module_part
ctx.params.update({"func": func_part})
ctx.params["path_or_module"] = path_or_module Hmm and I’d add an OS check to that condition gate too |
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 for your work on this @lmmx!
I've thought about this for a bit, and to be completely honest I'm a little on the fence about having this feature. Mostly because there is already a valid way to achieve what you want. While I can see the appeal of using the :
syntax, that also makes the code more verbose and error-prone, as exemplified by the fact that we need to think specifically about Windows paths etc.
I'll solicit feedback from Tiangolo to get his opinion. If he does want to support this, we'll still need to make sure the edits for the Window paths get added & tested.
Adds support for specifying functions directly using colon syntax in the typer CLI, as used in pyproject console scripts entrypoint syntax, and projects like uvicorn/gunicorn.
Described in discussion #1274
Before:
After:
The change is pretty minimal - 4 lines added to
maybe_update_state()
to parse the colon syntax and set the function parameter on the click Context params dict which is then picked up in the existing part of the function afterwards.Changes:
"module:function"
and"/path/to/file.py:function"
syntax intyper.cli:maybe_update_state