Re: [PATCH 2/2] bash-completion:fix shellcheck warning

[Fengtao (fengtao, Euler)]

On 2022/11/24 0:09, Daniel Kiper wrote:
> On Mon, Sep 19, 2022 at 09:20:14AM +0800, t.feng via Grub-devel wrote:
>> SC2207 (warning): Prefer mapfile or read -a to split
>> command output (or quote to avoid splitting).
>> SC2120 (warning): __grub_get_options_from_help references arguments,
>> but none are ever passed.
>> SC2155 (warning): Declare and assign separately to avoid
>> masking return values.
>>
>> In grub-completion.bash.in line 56:
>>         COMPREPLY=($(compgen -P "${2-}" -W "${1-}" -S "${4-}" --
>> "$cur"))
>>                    ^-- SC2207 (warning)
>>
>> In grub-completion.bash.in line 63:
>> __grub_get_options_from_help () {
>> ^-- SC2120 (warning)
>>
>> In grub-completion.bash.in line 115:
>>     local config_file=$(__grub_dir)/grub.cfg
>>           ^---------^ SC2155 (warning)
>>
>> In grub-completion.bash.in line 119:
>>         COMPREPLY=( $(compgen \
>>                     ^-- SC2207 (warning)
>>
>> In grub-completion.bash.in line 126:
>>     local grub_dir=$(__grub_dir)
>>           ^------^ SC2155 (warning)
>>
>> In grub-completion.bash.in line 128:
>>     COMPREPLY=( $( compgen -f -X '!*/*.mod' -- "${grub_dir}/$cur" | {
>>                 ^-- SC2207 (warning)
>>
>> SC2120: the current code meets the exception and does not need to be
>> modified
>>
>> ref:https://github.com/koalaman/shellcheck/wiki/SC2207
>> ref:https://github.com/koalaman/shellcheck/wiki/SC2120
>> ref:https://github.com/koalaman/shellcheck/wiki/SC2155
> 
> I think this should be split into three patches.
> 
> And again missing SOB...
> 

ok, I will fix this.

>> ---
>>  .../bash-completion.d/grub-completion.bash.in | 40 ++++++++++++-------
>>  1 file changed, 25 insertions(+), 15 deletions(-)
>>
>> diff --git a/util/bash-completion.d/grub-completion.bash.in 
>> b/util/bash-completion.d/grub-completion.bash.in
>> index 93d143480..7449e629a 100644
>> --- a/util/bash-completion.d/grub-completion.bash.in
>> +++ b/util/bash-completion.d/grub-completion.bash.in
>> @@ -53,7 +53,10 @@ __grubcomp () {
>>          ;;
>>      *)
>>          local IFS=' '$'\t'$'\n'
>> -        COMPREPLY=($(compgen -P "${2-}" -W "${1-}" -S "${4-}" -- "$cur"))
>> +        COMPREPLY=()
>> +        while read -r line; do
>> +            COMPREPLY+=("${line}")
>> +        done < <(compgen -P "${2-}" -W "${1-}" -S "${4-}" -- "$cur")
>>          ;;
>>      esac
>>  }
>> @@ -112,28 +115,35 @@ __grub_get_last_option () {
>>
>>  __grub_list_menuentries () {
>>      local cur="${COMP_WORDS[COMP_CWORD]}"
>> -    local config_file=$(__grub_dir)/grub.cfg
>> +    local config_file
>> +    config_file=$(__grub_dir)/grub.cfg
>>
>>      if [ -f "$config_file" ];then
>>          local IFS=$'\n'
>> -        COMPREPLY=( $(compgen \
>> -            -W "$( awk -F "[\"']" '/menuentry/ { print $2 }' $config_file 
>> )" \
>> -            -- "$cur" )) #'# Help emacs syntax highlighting
>> +        COMPREPLY=()
>> +        while read -r line; do
> 
> This looks strange. The shellcheck suggest "read -a" and you use "read -r" 
> here.
> Could you explain that?
> 

I print the compgen  -W "$( awk -F "[\"']" '/menuentry/ { print $2 }' 
$config_file )" -- "$cur"
It was multiple lines like:
        --id
        EulerOS (4.19.90-vhulk2111.1.0.h889.eulerosv2r10.aarch64) 2.0 (SP10)
        EulerOS (0-rescue) 2.0 (SP10)
        System setup
So, ref to: https://github.com/koalaman/shellcheck/wiki/SC2207, if it outputs a 
line with multiple
words, it should use read -a and if it outputs multiple lines, we can use 
mapfile or read -r with a loop.
Anyway, we can make a easy test:
        # IFS='\n' read -r -a array <<< "$(echo -e "1\n2\n3\n")"
        # echo ${array[*]}
        # 1  <<< this is incorrect
---------------------------------------
        # array=();while IFS='\n' read -r line; do array+=("$line"); done < 
<(echo -e "1\n2\n3")
        # echo ${array[*]}
        # 1 2 3  <<< this is right

Final, SC2207 said use mapfile in bash 4.4+, and read -r with a loop will be ok 
in bash 3.x+
So I choice read -r.(I did not test mapfile)

>> +            COMPREPLY+=("${line}")
>> +        done < <(compgen \
>> +                -W "$( awk -F "[\"']" '/menuentry/ { print $2 }' 
>> $config_file )" \
>> +                -- "$cur" ) #'# Help emacs syntax highlighting
>>      fi
>>  }
>>
>>  __grub_list_modules () {
>> -    local grub_dir=$(__grub_dir)
>> +    local grub_dir
>> +    grub_dir=$(__grub_dir)
>>      local IFS=$'\n'
>> -    COMPREPLY=( $( compgen -f -X '!*/*.mod' -- "${grub_dir}/$cur" | {
>> -         while read -r tmp; do
>> -             [ -n "$tmp" ] && {
>> -                 tmp=${tmp##*/}
>> -                 printf '%s\n' ${tmp%.mod}
>> -             }
>> -         done
>> -         }
>> -        ))
>> +    COMPREPLY=()
>> +    while read -r line; do
>> +        COMPREPLY+=("${line}")
>> +    done < <(compgen -f -X '!*/*.mod' -- "${grub_dir}/$cur" | {
>> +        while read -r tmp; do
>> +            [ -n "$tmp "] && {
> 
> It seems to me last '"' is in wrong place.
> 

I will fix this :)

>> +                tmp=${tmp##*/}
>> +                printf '%s\n' ${tmp%.mod}
>> +            }
>> +        done
>> +    })
>>  }
> 
> Daniel
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>