search

carvel-dev / vendir

Easy way to vendor portions of git repos, github releases, helm charts, docker image contents, etc. declaratively
https://carvel.dev/vendir
Apache License 2.0
289 stars 52 forks source link

Bash completion error #275

Open MAHDTech opened 1 year ago

MAHDTech commented 1 year ago

What steps did you take:

source <(vendir completion bash)

What happened:

source <(vendir completion bash)
bash: Succeeded: command not found
echo $?
127

What did you expect:

source <(vendir completion bash)
echo $?
0

Anything else you would like to add:

A minor issue for the team is when loading bash completion, the word "Succeeded" is printed to stdout as per here.

When sourced as shown above, this causes bash to try and run Succeeded as a command resulting in

bash: Succeeded: command not found

A workaround is as follows but would be nice to be the same as other carvel tools.

source <(vendir completion bash | grep -v "Succeeded")

Environment:

PRETTY_NAME="Debian GNU/Linux 12 (bookworm)"
NAME="Debian GNU/Linux"
VERSION_ID="12"
VERSION="12 (bookworm)"
VERSION_CODENAME=bookworm
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"

Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible" 👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help working on this issue.

praveenrewar commented 1 year ago

Hey @MAHDTech! Thanks for pointing this out. The completion command is the courtesy of cobra, we didn't add it manually and so didn't notice that this issue is present in vendir as well (similar to kapp and imgpkg where it has been fixed). Would you like to make a PR with a fix similar to this? There is just one catch, we need to add the "completion" command here before we can use IsCobraManagedCommand function, so we will need 2 PRs to fix this (the first one in cppforlife/cobrautil)