bash: fix shellcheck errors #889
Conversation
bash_completions.go
Outdated
| @@ -72,7 +72,7 @@ __%[1]s_handle_reply() | |||
| else | |||
| allflags=("${flags[*]} ${two_word_flags[*]}") | |||
| fi | |||
| COMPREPLY=( $(compgen -W "${allflags[*]}" -- "$cur") ) | |||
| mapfile -t COMPREPLY < <(compgen -W "${allflags[*]}" -- "$cur") | |||
There was a problem hiding this comment.
mapfile is not present in all bash versions. Namely, the out-of-the-box bash for MacOS doesn't have it (it's at a 3.0 version, it looks like mapfile was introduced in 4.0).
read -a COMPREPLY looks like it'll do the trick
There was a problem hiding this comment.
I think i need a project where i can test this. Disabling escape characters using -r (shellcheck warning) and the IFS seems to be required as well for it to work. So it's probably one of:
# For bash 3.x+
array=()
while IFS='' read -r line; do array+=("$line"); done < <(mycommand)
or
# For bash
IFS=" " read -r -a array <<< "$(mycommand)"
Some tests with read -a in a bash shell didn't give me the right result (or i simply used it wrong).
There was a problem hiding this comment.
Yeah it seems read -a really doesn't work as expected. I've tested this on multiple bash versions, and it seems to be the most consistent:
while read -s -d " " c ; do
COMPREPLY+=("$c")
done < <(compgen -W "${allflags[*]}" -- "$cur")
There was a problem hiding this comment.
Does it need to be cleared first?: COMPREPLY=()
There was a problem hiding this comment.
Bash will automatically create the array variable if you start appending to it:
unset a
a+=("foo")
echo "${a[@]}"
foo
But it wouldn't hurt to make sure it's empty anyway
There was a problem hiding this comment.
Was just worried that it is defined before already, but should be fine then.
Just compiled https://github.com/gohugoio/hugo with it but seems there is still sth. wrong with the completion (https://gist.github.com/rsteube/7647a9a3456cdb102f0b88ba004d74f8).
Currently looking into it.
There was a problem hiding this comment.
while IFS='' read -r c; do works in my shell (5.0.7).
https://gist.github.com/rsteube/6bb3336adf902193997bba048094b870
There was a problem hiding this comment.
3.2.57 looks ok as well: https://hub.docker.com/r/albers/bash-completion-mac
|
@rsteube, while I review this, can you squash your commits? |
|
@jharshman done |
|
@rsteube thanks, I'm just going to do a bit of testing of the bash completions before I merge this in. I would encourage you to do the same if you haven't yet. I'm not seeing any breaking changes here but just to be sure. |
|
Recompiled hugo with the squashed version and no changes to https://gist.github.com/rsteube/6bb3336adf902193997bba048094b870 which seemed to be fine in bash |
|
Validated by recompiling kubectl. |
bash: fix shellcheck errors (spf13#889)
bash: fix shellcheck errors (spf13#889)
PR spf13#889 introduced a regression where the global variable $c is no longer set when *custom_func is called. This is because $c is re-used by mistake in the read loop. This PR simply changes the name of the variable used in the loop. Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
PR #889 introduced a regression where the global variable $c is no longer set when *custom_func is called. This is because $c is re-used by mistake in the read loop. This PR simply changes the name of the variable used in the loop. Signed-off-by: Marc Khouzam <marc.khouzam@gmail.com>
Fixed shellcheck errors according to documentation:
https://github.com/koalaman/shellcheck/wiki/SC2207
https://github.com/koalaman/shellcheck/wiki/SC2164
untested - tests are ok but someone should verify this is correct