Open tiangolo opened 2 months ago
After reviewing click 7/8 and how I'm using shell_complete downstream I have the following recommendations:
1) Allow completer functions to return lists of CompletionItems, strings or 2-tuples of completions and helps. CompletionItem may change over time and there's no harm in passing them through if user functions return them. (my downstream functions already do return these)
2) Deprecate args in favor of click.core.Parameter. The parameter class is helpful for defining generic completer functions that otherwise would not know which parameter they are attached to. I'm using the parameter class downstream to weed out duplicates in multi value argument completions and to know if the parameter value was a default or not:
if (
not allow_duplicates
and param.name
and ctx.get_parameter_source(param.name) is not ParameterSource.DEFAULT
):
present = [value for value in (ctx.params.get(param.name) or [])]
I think args is already not backwards compatible with how it behaved in Click 7 It was also not very useful in Click 7. It resolved to the list of arguments passed through the environment variables used by the different shell completion entry point scripts and was also subject to additional ad hoc processing depending on the shell. Reverting it to the Click 7 behavior would be difficult and also probably not worth it because again, it wasn't very useful. It is also not equivalent to ctx.args.
The Parameter class in concert with the Context however gives you access to how click interpreted the complete CLI parameters before the incomplete string.
I think the best thing to do would be to allow completer functions to specify args: List[str] and/or param: click.core.Parameter
, but throw a deprecation warning if args is present and pass an empty list. I really doubt theres a lot of existing code making use of args out there because its not very useful (you'd have to do a lot of manual processing in many cases) and also I think its already broken and nobody seems to be complaining. Parameter on the other hand is much more useful. So following this recommendation would not break existing code more than it already is and allow graceful transition to using Parameter instead.
There is a way to support the old args: List[str]
parameter. This and the other suggestions (minus the deprecation warning) are submitted here: https://github.com/svlandeg/typer/pull/1
Privileged issue
Issue Content
In Short
We want a single parameter to handle CLI parameter autocompletion, based on type annotations.
Current State
Right now the Typer docs only talk about the function for the parameter
autocompletion
.And Typer has (proper) support for that function based on type annotations.
It checks the types for a
Context
,list[str]
for other args, andstr
for the current incomplete args: https://typer.tiangolo.com/tutorial/options-autocompletion/#types-types-everywhereThe support for autocompletion is based on the type annotations (like the rest of Typer), not on the order or name of the arguments.
Now with Click 8.x the underlying parameter
autocompletion
was removed and there's a new oneshell_complete
(https://click.palletsprojects.com/en/8.1.x/api/#click.ParamType.shell_complete).In Typer,
autocompletion
raises a warning already: https://github.com/fastapi/typer/blob/master/typer/core.py#L65But there are no docs nor proper support for
shell_complete
based on type annotations.We Want
We want to have support for completion based on type annotations. The parameter should be able to expect a
Context
and astr
with the incomplete parameter. But I'm not sure of the parameter name for this function.Note: we probably don't need to support
list[str]
for extra args as that's available fromcontext.args
.We should have that as the main documented feature. We could have a note at the end mentioning that there's a (now deprecated)
autocompletion
parameter, with the basics of how it worked, but the main functionality would be with the new parameter.Hardest Problem in CS: Naming
Writing this I'm realizing that in Typer I'm using the same parameter name that was available in Click:
autocompletion
, but in reality, it has different semantics.In Click, the meaning of the parameters is based on the position/order.
In Click 7.x it had to be in the exact order of
ctx
,args
,incomplete
:In Click 8.x it has to be the exact order of
ctx
,param
,incomplete
:In Typer,
autocompletion
it's based on the type annotations, which also means that the order doesn't matter, if all of the parameters are present or not doesn't matter, the name of the parameters doesn't matter. The only thing that matters is the type annotations: https://typer.tiangolo.com/tutorial/options-autocompletion/#types-types-everywhereThis means that the name of the parameter
autocompletion
is the same as in Click, but the semantics are actually not the same. :thinking:That makes me think that maybe the parameter should have a different name, so that people don't get confused with it assuming it's the same as in Click, and it really is not, at least not in the same way.
Maybe we should just keep the same name
autocompletion
just because that's the main name we've had documented in Typer. :thinking:My original thought was to support a parameter
shell_complete
(like the one in Click) but based on type annotations. But now thinking about it, I think maybe it makes more sense to have a different name (or just keep the oldautocompletion
name). And have it based on type annotations.I think the new/final parameter should be able to request (via type annotations) the
Context
and theincomplete
str
. I don't think we should support it requesting theparam
ClickParameter
until we find a reason to do so (I can't find any yet).The other thing is, the old Click
autocompletion
supported returningstr
values or tuples. The newshell_complete
expects objects of typeCompletionItem
, but I wouldn't think requiring users to import that class and create instances of it would be a great developer experience, so I would prefer to keep supporting the older syntax too.Note on
CompletionItem
From the source, I understand that the main thing it provides apart from the previously supported data is a new
type
that tells the completion parts if the thing to complete is a directory or file... I'm not sure in which cases that type is useful, maybe internally, but not sure for completion functions. And also, if there's a use case where that makes sense, we can probably know the type from the type annotation (e.g.Path
).