-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Experimental π§ π· π·ββοΈ: Initial plugins implementation #11680
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
Experimental π§ π· π·ββοΈ: Initial plugins implementation #11680
Conversation
β¦this change, update tests that call the generate_parser() function
Co-authored-by: Filipe Lains <lains@riseup.net>
Co-authored-by: Filipe Lains <lains@riseup.net> Co-authored-by: Katherine Kinnaman <kkinnaman@anaconda.com>
Co-authored-by: Filipe Lains <lains@riseup.net>
Co-authored-by: Filipe Lains <lains@riseup.net>
β¦cs configuration for custom internal links
Co-authored-by: Travis Hathaway <travis.j.hathaway@gmail.com>
Co-authored-by: Travis Hathaway <travis.j.hathaway@gmail.com>
Co-authored-by: Travis Hathaway <travis.j.hathaway@gmail.com>
Co-authored-by: Travis Hathaway <travis.j.hathaway@gmail.com>
Co-authored-by: Katherine Kinnaman <kkinnaman@anaconda.com>
Co-authored-by: Katherine Kinnaman <kkinnaman@anaconda.com>
Co-authored-by: Katherine Kinnaman <kkinnaman@anaconda.com>
β¦comprehension, find all possible subcommand conflicts
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.
FWIW the idea for the initial plugin implementation was to stay close to the previous conda-*
API and have a similar behavior and specifically not provide an argparse
specific plugin hook.
That was a design decision to reduce the effort needed for existing packages extending conda with conda-*
scripts to have a transition path and gain some experience before we support more advanced techniques like supporting click etc.
I've left a few comments and think this still has value FWIW, e.g. we could move this into an own ArgparseCondaSubcommand
class that could be an βadvancedβ option for plugins with more complex needs. That said, generally speaking, I would caution us to leaning into argparse
given its obvious API problems. I'd rather have us work towards supporting tools like click or typer.
conda/cli/conda_argparse.py
Outdated
raise CondaError( | ||
"There is something mis-configured with your subcommands. This could be due to an " | ||
"incorrectly configured plugin. Please try uninstalling any problematic plugins " | ||
"you may have to remove this error." |
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.
Would it help to print the args
here so users can identify what is not working?
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.
Yeah, definitely would be nice to have some detailed debugging information here just like the error report we normally show.
This time we have an implementation that fully supports any CLI parsing library you may want to use. It does by modifying the ArgumentParser class to add a "pass-thru" for plugin subcommands. This simply means that we add a stub for the command and then pass it off to the plugin for actual command line argument parsing.
β¦should fix that
@travishathaway is it okay to close this? |
This pull request was largely inspired by this pull request: #11435
What I am trying to do differently here is to give plugin authors a way to define a command line interface for their sub-commands. This is accomplished by telling
argparse
inconda
not to parse arguments for sub-commands it knows to belong to plugins.This is accomplished by overriding the
parse_args
method on our customArgumentParser
class:https://github.com/travishathaway/conda/blob/9277f1e439f890bbbf557499d4045f646bb76034/conda/cli/conda_argparse.py#L228-L236
If it's a known plugin, we simply do nothing. Under the hood, it simply checks the value of
sys.argv[1]
to see if that string matches a name in our registered plugins list.Later in
do_call
:https://github.com/travishathaway/conda/blob/9277f1e439f890bbbf557499d4045f646bb76034/conda/cli/conda_argparse.py#L107-L113
We perform a naive check (maybe there's better way to do this?) to see that the
args
object doesn't have afunc
attribute. If doesn't we proceed to find the sub-command and execute its action callable.Because this just calls a function without having parsed any arguments thus far, this leaves us free to do whatever kind of argument parsing we want. Below is an example program using
click
as the argument parser:The
click
program needs to be setup as-if it was a program using sub-commands which is why we define aclick.group
. Otherwise, it works just like you would expect it.