Open epage opened 2 years ago
Had a few busy weeks but I'm back to working on this now.
After trying to fit zsh support into the existing code, I realized that different shells might need to support a very different set of command line options for their completions and registration commands. This prompted me to split the complete command and the register command into different subcommands, with the shell as either a value (for registration) or as a subcommand that can define its own options for each shell. Following cobra's example, I've also prefixed the complete command with underscores to mark it as an internal command not for use by the consumer.
Here's how usage would look now:
# Write the bash completion script to stdout
dynamic completions bash -
# Process completions for bash (called from the completion script)
dynamic __complete bash --index=0 --type=9 -- a b c
# Write the zsh completion script to completions.zsh
dynamic completions zsh completions.zsh
This results in a much better layout, where generic completion code can live inside clap_complete/src/dynamic/complete.rs
, and the shell specific code in clap_complete/src/dynamic/{zsh,bash,...}.rs
calls out to it after parsing options. dynamic/mod.rs
pretty much just defines the subcommand and then delegates to the shell specific code.
Are you alright with changing the subcommand like this, or is there a strong requirement to keep the existing structure? (e.g. a single complete
command with a --register
switch and all options defined in one place)
Could you go into more detail for as to why zsh needs something different?
My biggest concern with splitting things out is ensuring users can add support for shells we don't officially support. The second is that the interface can have some complications to it and would prefer to have as much unified as possible to avoid duplicating problems.
Sure thing! When trying to adapt the cobra zsh completion script, I noticed that no IFS
argument is used, COMP_CWORD
is not offered by zsh, and COMP_TYPE
is not expressed as an integer. In the case of COMP_TYPE
for example, we would either need to convert the int (e.g. 9, 33, etc.) in the bash script (which is what cobra does), or convert to the respective bash int for other shells. Both solutions don't feel good to me, but I could understand going with the first one.
My solution would definitely lead to some code duplication. However, I feel like it would actually make it easier to implement support for new shells, since there's no chance of implementation details of one shell leaking out into others. When implementing a new shell, you get functions that will complete a given input, a framework for how to call the completion function, and you would just have to "fill in the middle part" that links the two.
I could understand if you would prefer an abstraction that covers the largest amount of space to keep duplication to a minimum, though.
COMP_CWORD is not offered by zsh
Looks like CURRENT
is used. How is it different than COMP_CWORD
?
COMP_TYPE is not expressed as an integer
I'm fine changing how we express it or moving it into the bash layer. My overall hope was that the core completion logic would be shell agnostic and the registration code would bridge between clap and the shell as needed.
However, I feel like it would actually make it easier to implement support for new shells
My concern for supporting other shells is the composability of the API. Since we don't have alternative shells, we don't have much for that today but its something we need to be able to handle as we do support more shells.
However, I feel like it would actually make it easier to implement support for new shells, since there's no chance of implementation details of one shell leaking out into others. When implementing a new shell, you get functions that will complete a given input, a framework for how to call the completion function, and you would just have to "fill in the middle part" that links the two.
Isn't the function you described the complete
function today? My hope is that the rest on top can work in the 99% case to not require a lot of boiler plate for users and developers. It does shift some responsibility to the bridge code in the script..
Looks like CURRENT is used. How is it different than COMP_CWORD?
Yes, my bad. I overlooked that, you're correct on that one.
I'm fine changing how we express it or moving it into the bash layer. My overall hope was that the core completion logic would be shell agnostic and the registration code would bridge between clap and the shell as needed.
Moving that into the completion registration script is absolutely an option. I'm just wary of a future where two sets of flags used by two different shells are completely disjunct. But I also concede that most shells we would implement right now have a large enough overlap to justify trying to make the code agnostic.
Isn't the function you described the complete function today? My hope is that the rest on top can work in the 99% case to not require a lot of boiler plate for users and developers. It does shift some responsibility to the bridge code in the script..
Yes, it is. The thing I was advocating for is essentially allowing each shell to define:
flatten
ed), andcomplete
function.My hope would be that this would keep quirks of specific shells from leaking out to the completion code.
Maybe I'm not doing a good job of describing what I'm proposing. I'll try to improve the model implementation I was working on and see how much duplication that would actually result in and what the resulting API would look like. That wouldn't be much work and then we'll actually have something tangible to discuss.
I don't quite understand the role of COMP_TYPE
, is it used to set the triggering way of completion? Maybe we can ignore it and focus more on what the complete
function needs, like COMP_WORDS
and COMP_CWORD
in bash.rs. Also I don't seem to find any built-in variables similar to COMP_TYPE
in zsh. I think that I could work on this issue.
Following cobra's example, I've also prefixed the complete command with underscores to mark it as an internal command not for use by the consumer.
I think that this could be a thing, because users may need the complete
subcommand.
Also in https://github.com/clap-rs/clap/blob/70e84174a72f766020f5f65f354e16f1e1a49bed/clap_complete/src/dynamic/shells/bash.rs#L30, we should export _CLAP_IFS
rather than IFS
See #3166 for more context
Tasks