buildah
buildah copied to clipboard
WIP: switch to automatically-generated completions
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Get out of business of manually crafting and updating shell completion helper functions. Fixes #6069
How to verify it
I don't use a shell that benefits from this (old habit), but if you do, source the new file and see if it works?
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Shell completions are now generated rather than manually crafted, so completion suggestions will look different.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: flouthoc, nalind
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [flouthoc,nalind]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Also I guess this should have a Fixes https://github.com/containers/buildah/issues/6069?
Ok I did some local tests I think we might one to port some other changes from podman such as https://github.com/containers/podman/blob/badf6b8b1735dff7cdb45636eb8eba490454f9d9/cmd/podman/root.go#L311-L317
$ bin/buildah rm --
--all (remove all containers)
--cgroup-manager (cgroup manager)
--help (help for rm)
--log-level (the log level to be used, one of "trace", "debug", "info", "warn", "error", "fatal", or "panic")
--registries-conf-dir (path to registries.conf.d directory (not usually used))
--registries-conf (path to registries.conf file (not usually used))
--root (storage root dir)
--runroot (storage state dir)
--short-name-alias-conf (path to short name alias cache file (not usually used))
--storage-driver (storage-driver)
--storage-opt (storage driver option)
--userns-gid-map (default `ctrID:hostID:length` GID mapping to use)
--userns-uid-map (default `ctrID:hostID:length` UID mapping to use)
Because the are persistent flags they can be set technically anywhere but realistically for a user suggesting them on each command makes no sense.
Second as I though just doing buildah rmi no longer completes the image ids/names which is a clear UX regression so I rather see that implemented before we switch the script.
I am happy to take that over if you don't want to spend the time understanding all the small details in play here, its something I can do while being offline on the train next week.
I am happy to take that over if you don't want to spend the time understanding all the small details in play here, its something I can do while being offline on the train next week.
If you want to, sure, otherwise I could stand to get more familiar with those bits.
Ephemeral COPR build failed. @containers/packit-build please check.
/retitle WIP: switch to automatically-generated completions
If you want to, sure, otherwise I could stand to get more familiar with those bits.
I find other stuff to do, if you want to have some deeper understanding please do that.
https://github.com/containers/podman/blob/badf6b8b1735dff7cdb45636eb8eba490454f9d9/cmd/podman/root.go#L302-L321
This must be done in persistentPreRunE() function that is registered on the root command here in buildah in a similar way, I see there is already a before() function where it could be added. First it is used to hide the top level flags on subcommand so they don't show up all the time. The cmd.Root().Traverse(os.Args[2:]) call is not right in the context of buildah I think, podman has some really special cli setup where this hack was needed but I don't know if it is needed for buildah. Traverse() is definly wrong because that must only be used when TraverseChildren is set to true which it is not for buildah so it would need to be Find() if it is needed at all.
I guess the easier way to check if the command being pass in is the root buildah command is to just compare the pointer of cmd against rootCmd.
For actual specific completion suggestions there is https://github.com/containers/podman/blob/badf6b8b1735dff7cdb45636eb8eba490454f9d9/cmd/podman/common/completion.go with a ton of example functions.
Than you just have to register them to the command via ValidArgsFunction or in case of flags RegisterFlagCompletionFunc(),
And for testing with bash source <(bin/buildah completion bash) will work.
The nice thing about the binary is generating the completions is if you build buildah from source and let's say added a new command then it will work right away without having to reload the script, bin/buildah [TAB] would show it while /usr/bin/buildah [TAB] will not since that binary does have the command yet.
PR needs rebase.
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-sigs/prow repository.
A friendly reminder that this PR had no activity for 30 days.