[WIP] autogenerate bash completions

[lsm5]

Signed-off-by: Lokesh Mandvekar [email protected]

[rhatdan]

Should we just remove the bash completions and always generate them?

[lsm5]

Should we just remove the bash completions and always generate them?

I'm ok with that, it's just that I noticed podman's completions were included in the source itself.

[lsm5]

I'm guessing it's ok to document the completion command? Anyone think otherwise?

[lsm5]

Also, do we need zsh and fish while we're at it? I noticed the installation only mentioned bash.

[lsm5]

The current install-completions target only install bash completion. Would it be ok to rename it to install-bash-completions? I'm guessing it would break quite a few users.

[rhatdan]

@Luap99 PTAL

[lsm5]

I still need to resolve:

$ make validate-docs
hack/man-page-checker
whack/xref-helpmsgs-manpages
xref-helpmsgs-manpages: skopeo completion --help lists bash, but bash not in docs/skopeo-completion.1.md
xref-helpmsgs-manpages: skopeo completion --help lists fish, but fish not in docs/skopeo-completion.1.md
xref-helpmsgs-manpages: skopeo completion --help lists powershell, but powershell not in docs/skopeo-completion.1.md
xref-helpmsgs-manpages: skopeo completion --help lists zsh, but zsh not in docs/skopeo-completion.1.md
make: *** [Makefile:250: validate-docs] Error 1

@edsantiago thoughts?

[edsantiago]

I would suggest making skopeo completion a hidden option, one that does not show up under skopeo help.

[lsm5]

I would suggest making skopeo completion a hidden option, one that does not show up under skopeo help.

are there any issues to exposing the completion subcommand to users?

[edsantiago]

Podman hides it

[lsm5]

i gotta rework this to hide the completion subcommand.

[mtrmac]

Thanks!

---

Should we just remove the bash completions and always generate them?

I’d weakly prefer that; we must not change the generated code, and we are not going to realistically review it in detail.

AFAICS the cost of having the committed in the repo would be to add extra tasks and extra churn on possible re-vendors of the CLI library (and extra worry about getting that right so that the file doesn’t accidentally get stale) — at the benefit of possibly making cross-architecture builds a bit easier, if anyone ever does that.

---

Also, do we need zsh and fish while we're at it? I noticed the installation only mentioned bash.

We do need zsh (the default shell on macOS). I have no idea how widespread or relevant fish is.

---

After this infrastructure, the more interesting (?) part is making the completions good/magical.

Notably the manually-written script seems to be able to suggest transport names; preserving that in this PR (for the relevant arguments only?) would be very nice.

Any other additions, like perfecting completion for all the argument values, with everything https://github.com/spf13/cobra/blob/master/shell_completions.md allows, probably should be separate PRs later.

[mtrmac]

Why should any of these be .PHONY dependencies? AFAICS the target is a real file that depends only on a real file.

[lsm5]

ack, noted.

[mtrmac]

It seems simpler to just install all of them in install-completions. Who has so little space and so much time to choose individually?

[Luap99]

The reason the completion command is hidden in podman is because I did not want podman com[TAB] to complete both commit and completion. The completion has really no interactive use cases except testing. If a command is hidden it will not be included in the shell completions. Skopeo has a similar problem with the skopeo copy command. I want skopeo c[TAB] to complete to skopeo copy and not to skopeo co and then waiting for you to type the next character.

Either way you should document this command in a man page. Your current page has not much information on how to use this command. I recommend to copy and adapt the podman page: https://github.com/containers/podman/blob/main/docs/source/markdown/podman-completion.1.md

Also right now you are using the default included completion command. This has some problems:
First, you cannot hide it.
Second, the command is structured as a subcommand which has a command for each shell. This doesn't make a difference in how you would call it as users but the man page checker would expect a man page for skopeo completion (just a subcommand) and then 4 pages for each shell skopeo completion {bash,zsh,fish,powershell}.

To fix these issues I would recommend you to use the same command structure as in podman: https://github.com/containers/podman/blob/main/cmd/podman/completion/completion.go It would be great if we could have consistency with that. IF you want to use that I think we should add this command to c/common here https://github.com/containers/common/tree/main/pkg/completion and then import it into podman/skopeo.

Also, do we need zsh and fish while we're at it? I noticed the installation only mentioned bash.

We do need zsh (the default shell on macOS). I have no idea how widespread or relevant fish is.

Yes please enable all shells. Having this enabled for the other shell requires exactly zero additional maintenance. That is the beauty with the cobra completion logic in golang.

After this infrastructure, the more interesting (?) part is making the completions good/magical.

Notably the manually-written script seems to be able to suggest transport names; preserving that in this PR (for the relevant arguments only?) would be very nice.

Any other additions, like perfecting completion for all the argument values, with everything https://github.com/spf13/cobra/blob/master/shell_completions.md allows, probably should be separate PRs later.

This is the difficult and tedious part. Basically every command and flag will need a ValidArgsFunction to provide customized completion. The current default in skopeo is to provide no completion for flags and commands besides the transpart name completion AFAIK. With this change we get the default shell completion (e.g. path names will get completed). This makes sense for some flags --authfile but not for others --format. To fix this you would need to a a completion function to every single flag. In https://github.com/containers/common/blob/main/pkg/completion/completion.go you already have AutocompleteNone to provide no completion for this flag/command and AutocompleteDefault to provide the path completion for this flag/command.

This took a very long time in podman but was definitely worth it. There is also a test to ensure every command and flag has such function set https://github.com/containers/podman/blob/main/cmd/podman/shell_completion_test.go

I do not think this is required for this PR but I aggre with @mtrmac that we should try to preserve the current behaviour (complete transport names and maybe more?) to not cause regression for users.

[Luap99]

FYI This is the PR for podman containers/podman#6442

[rhatdan]

I agree the command should be hidden. We don't want general users playing with it.

[mtrmac]

Either way you should document this command in a man page. Your current page has not much information on how to use this command. I recommend to copy and adapt the podman page: https://github.com/containers/podman/blob/main/docs/source/markdown/podman-completion.1.md

I am not familiar with all the shells, but at least the bash and zsh experience suggests users don’t quite “use” the completion command; they enable completion as a general matter, and the various software drops down files into the completion-infra-provided directory. So, wouldn’t this be completely user-transparent when shipped in a RPM, or a part of make install or perhaps the Skopeo installation instructions, for others?

Pointers for how to enable the bash-completion package in general are useful, and might be in a man page, sure, but it’s not completely obvious to me that those not-really-Skopeo-instructions about bash-completion, and a reference documentation for a hidden command that users should never need to call manually, should be in the same document.

---

Also right now you are using the default included completion command. This has some problems: First, you cannot hide it. Second, the command is structured as a subcommand which has a command for each shell. This doesn't make a difference in how you would call it as users but the man page checker would expect a man page for skopeo completion (just a subcommand) and then 4 pages for each shell skopeo completion {bash,zsh,fish,powershell}.

My first impression is that making that an exception in the checker (or hiding the command entirely, which we want to do anyway I think, and having the checker ignore it by general rule) would be simpler, and would make it easier to benefit from any possible future improvements to the built-in command.

I.e. the train of thought is that if we don’t place useful information in a man page for that subcommand, we don’t need to manage what that subcommand looks like in a man page, and so we don’t need special code to implement that subcommand. I like both the starting and ending point :) but I could well be missing something.

[github-actions]

A friendly reminder that this PR had no activity for 30 days.

[rhatdan]

@lsm5 What is going on with this PR?

[mtrmac]

Also right now you are using the default included completion command. This has some problems: First, you cannot hide it.

https://github.com/spf13/cobra 1.3.0 now provides

rootCmd.CompletionOptions.HiddenDefaultCmd = true

[Luap99]

I would like to take this over and push it over the finish line. I already created containers/common#1043 so that podman, skopeo and buildah could share the same completion command.

[lsm5]

@Luap99 yes, please go ahead. Thanks a lot!

[mtrmac]

Actually, the other PR was just merged, I’m afraid I have forgotten to close this one.