Add fish shell completion

[saschagrunert]

Hey there, I added the fish shell completion to podman. I also thought about adding an auto generation (cobra supports it for zsh, bash and (soon) for fish) subcommand, that the files don't have to be manually maintained. What do you think?

[openshift-ci-robot]

Hi @saschagrunert. Thanks for your PR.

I'm waiting for a containers or openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[vrothberg]

/ok-to-test
/approve

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: saschagrunert, vrothberg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [vrothberg]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vrothberg]

Thanks a lot, @saschagrunert!

I also thought about adding an auto generation (cobra supports it for zsh, bash and (soon) for fish) subcommand, that the files don't have to be manually maintained. What do you think?

I see one issue with auto-generating those from cobra, namely that it doesn't work for argument completions but only for commands, sub-commands, flags, and options. But we also need to complete image names, containers names, pods, etc.; any kind of completion that requires semantic knowledge.

I think auto-generated files are nice as a starting point, but I don't think we can avoid manual maintenance afterward.

[vrothberg]

One nit, otherwise LGTM.

[TomSweeneyRedHat]

maybe I've missed something, but is 'generate-completion' valid?

[TomSweeneyRedHat]

Oh, I think I see now, it's for the shell completions itself, not necessarily a podman command.

[saschagrunert]

Yeah, sorry I forgot to remove it after my modification of podman 👍

[TomSweeneyRedHat]

The changes LGTM, but I'm very concerned about manually maintaining podman.fish over time. I can just see it getting out of step too quickly. I'd feel more comfortable with it if podman.fish could be created reliably in a make process based on the Cobra settings. Definitely want @mheon, @rhatdan and @baude to chime in on this one.

[saschagrunert]

We could wait for this one spf13/cobra#754 and then add necessary fish functions for the completion of, for example, image names. Then we would have an auto-generated and manually maintained part of the completion.

[edsantiago]

@saschagrunert have you looked at the zsh completion code? That's almost entirely self-discovering, using `podman --help', with a few extensions for determining image/container/volume/registry names. Perhaps something like that could be done with fish as well? As others have mentioned, hardcoding every option is not maintainable.

And, incidentally, you will need to update contrib/spec/podman.spec.in to get CI to pass. (never mind; I just saw your new force-push)

[saschagrunert]

@saschagrunert have you looked at the zsh completion code? That's almost entirely self-discovering, using `podman --help', with a few extensions for determining image/container/volume/registry names. Perhaps something like that could be done with fish as well? As others have mentioned, hardcoding every option is not maintainable.

Yeah I also thought about an on-the-fly parser, but in the end I see pros and cons for both kinds of solutions. Let me elaborate how such a parser can be implemented and then come back with my findings.

/hold

[saschagrunert]

@edsantiago I played around with some scripted (sed-based) approach in fish shell and must say it is damn slow (some seconds) compared to a generated file. I really would like to recommend in generating the completion grammatically and injecting necessary static functions as constant strings. The generation of the shell completion could then be done via some make target. WDYT?

I guess since the related PR in fish shell is not merged this can't be done here, too... 😟

[edsantiago]

Autogenerating a completion file at build time was actually my first approach; I abandoned it for several reasons including the difficulty of validating the generated results. I have to assume that your PR is mostly autogenerated; if you can come up with a reliable script that will run at build time, using only tools that are available on build systems, I'd be much more inclined to accept it enthusiastically. Thank you again for your efforts on this.

[saschagrunert]

I will close this for now and will follup up on this topic in the future.