-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow path completion for podman create/run --rootfs #9269
Allow path completion for podman create/run --rootfs #9269
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99 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:
Approvers can indicate their approval by writing |
LGTM |
cmd/podman/common/completion.go
Outdated
// if it is set to true we have to provide path completion | ||
rootfs, err := cmd.Flags().GetBool("rootfs") | ||
if err == nil && rootfs { | ||
return nil, cobra.ShellCompDirectiveDefault |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious: why not ShellCompDirectiveFilterDirs
? Can rootfs
ever be something other than a directory path? (e.g. tarball, image)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it has to be a directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, I completely forgot that this option exists.
If the --rootfs flag is set podman create/run expect a host path as first argument. The shell completion should provide path completion in that case. [NO TESTS NEEDED] This can manually be verified with `podman run --rootfs [TAB]`. Signed-off-by: Paul Holzinger <[email protected]>
33ec32f
to
1caace8
Compare
/lgtm |
Aw, phooey. I was just about to slash-lgtm when I remembered: this will have no effect unless we update the completion files themselves. @Luap99 since you're the only one who knows how to bring in the proper magic version of cobra, could you /hold |
Sorry, had to hold really fast ...could you do so and resubmit? |
PS |
/hold cancel @edsantiago There is no need to update the completion files. The completions files only needs to be updated for bug fixes in the scripts etc... For example the fish completion doesn't support |
If the --rootfs flag is set podman create/run expect a host
path as first argument. The shell completion should provide
path completion in that case.
[NO TESTS NEEDED]
This can manually be verified with
podman run --rootfs [TAB]
.