fix: unbound variables in bash completion #1321
Conversation
|
|
edentsai
force-pushed
the
fix-unbound-variable-in-bash-completion
branch
from
bc9e5d4
to
2bf2b31
Compare
|
This seems reasonable. |
Yes, those variables cause unbound variable warnings. |
|
I just checked the __start_kubectl()
{
# ...
local has_completion_function
local last_command
# ...
}
|
|
if you're saying tha last_command can never be bound then this isn't the right fix, is it? |
Sorry, I verified my guess is not correct. i just made a small script to test the case, |
|
I didn't dig deep whole flow about the kubectl completion, $ set -o nounset
$ kubectl <Tab>-bash: BASH_COMP_DEBUG_FILE: unbound variable
$# After fixed "$BASH_COMP_DEBUG_FILE" warning:
$ set -o nounset
$ kubectl <Tab>-bash: last_command: unbound variable# After fixed "$last_command" warning:
$ set -o nounset
$ kubectl <Tab>-bash: has_completion_function: unbound variablemaybe we can take a deep look how |
|
I found the root cause:
here is a simple script #!/usr/bin/env bash
__start_demo()
{
printf "[INFO] START\n"
# declare a variable WITHOUT value will cause unbound variable warning.
local last_command
printf "[INFO] last_command: ${last_command}\n"
printf "[INFO] END\n"
}
complete -o default -F __start_demo demothen use $ set -o nounset
$ source demo.sh
$ demo <Tab>[INFO] START
-bash: last_command: unbound variablejust declare the variable with an empty string to fix the issue: - local last_command
+ local last_command=""$ set -o nounset
$ source demo.sh
$ demo <Tab>[INFO] START
[INFO] last_command:
[INFO] END
# finally, the warning is fixed. |
edentsai
force-pushed
the
fix-unbound-variable-in-bash-completion
branch
from
2bf2b31
to
034c8aa
Compare
|
I updated the changes to declare |
|
I filed #1351 yesterday without being aware of this PR, sorry about that. However I think that one is more complete and subjectively better stylewise than the one here. But I don't really care that deeply how, as long as the issues get fixed. Just pointing that one out in case you wish to update this one with the missing parts (flagvalue, ZSH_VERSION) instead of adopting #1351. |
edentsai
force-pushed
the
fix-unbound-variable-in-bash-completion
branch
from
034c8aa
to
a267adb
Compare
|
I updated this PR:
|
Cool. BASH_VERSION was not in #1351 though, and I'm not sure why it is being checked for emptiness in the first place (maybe the intent is to support loading the bash completion in some non-bash shell?), but since the check is there, guarding it for undefinedness doesn't hurt much. It does have a bit of a code smell now that BASH_VERSION is being guarded, but BASH_VERSINFO is accessed unguarded. |
just treat BASH_VERSION like ZSH_VERSION,
humm. I think i should also guard the BASH_VERSINFO variable, |
when `set -o nounset` in Bash,
the warnings of unbound variables will break the bash completion.
use `kubectl` as example:
```sh
$ set -o nounset
$ my-cli <Tab>-bash: BASH_COMP_DEBUG_FILE: unbound variable
$
```
the warning break bash completion without any completion result,
and cause my cursor move to the newline.
Use `${variable:-}` substitution in Bash,
that assign an empty string as default for unbound variables to fix the warnings.
edentsai
force-pushed
the
fix-unbound-variable-in-bash-completion
branch
from
a267adb
to
72f71f5
Compare
|
PR updated. |
|
This PR is being marked as stale due to a long period of inactivity |
|
Bump |
|
Bump again. This MR already got stuck for a few months, |
|
Ping :) More duplicates are starting to crop up, e.g. #1441 |
|
Apologies for missing this PR for the 1.2 release. I'll try to get it added to the list for the next release. Also, I'll try to add a test for |
|
Cool to hear that such a repo exists 👍 Another similar option worth considering in the tests would be |
|
This PR is being marked as stale due to a long period of inactivity |
|
Hi @marckhouzam, anything I can do to help get this moving forward? |
|
Also, would be good if someone with powers could remove the stale label (unsure if something eventually closes stale bugs), but this isn't really stale anyway. |
I expect that once the next release of Cobra is planned, the maintainers will start merging things, so nothing to do right now. Thanks for your patience @scop |
when `set -o nounset` in Bash,
the warnings of unbound variables will break the bash completion.
use `kubectl` as example:
```sh
$ set -o nounset
$ my-cli <Tab>-bash: BASH_COMP_DEBUG_FILE: unbound variable
$
```
the warning break bash completion without any completion result,
and cause my cursor move to the newline.
Use `${variable:-}` substitution in Bash,
that assign an empty string as default for unbound variables to fix the warnings.
when `set -o nounset` in Bash,
the warnings of unbound variables will break the bash completion.
use `kubectl` as example:
```sh
$ set -o nounset
$ my-cli <Tab>-bash: BASH_COMP_DEBUG_FILE: unbound variable
$
```
the warning break bash completion without any completion result,
and cause my cursor move to the newline.
Use `${variable:-}` substitution in Bash,
that assign an empty string as default for unbound variables to fix the warnings.
when `set -o nounset` in Bash,
the warnings of unbound variables will break the bash completion.
use `kubectl` as example:
```sh
$ set -o nounset
$ my-cli <Tab>-bash: BASH_COMP_DEBUG_FILE: unbound variable
$
```
the warning break bash completion without any completion result,
and cause my cursor move to the newline.
Use `${variable:-}` substitution in Bash,
that assign an empty string as default for unbound variables to fix the warnings.
when
set -o nounsetin Bash,the warnings of unbound variables will break the bash completion.
use
kubectlas example:the warning break bash completion without any completion result,
and cause my cursor move to the newline.
Use
${variable:-}substitution in Bash ,that assign an empty string as default for unbound variables to fix the warnings.