[PATCH V2 0/4] bash-completion:fix shellcheck error and warning

[PATCH V2 0/4] bash-completion:fix shellcheck error and warning

[t.feng]

Hi,
The patch set fix some warning and error in grub-completion.bash.in.
And shellcheck also provides 'info' and 'style' level check, i think
grub do not need to modify.

shellcheck -s bash -S warning grub-completion.bash.in

shellcheck:https://github.com/koalaman/shellcheck

V2:
  split warnings patches
  fix error '"'

************************

t.feng (4):
  bash-completion:fix shellcheck error
  bash-completion:fix shellcheck SC2207-Warning
  bash-completion:fix shellcheck SC2155-Warning
  bash-completion:disable shellcheck SC2120-Warning

 .../bash-completion.d/grub-completion.bash.in | 41 ++++++++++++-------
 1 file changed, 26 insertions(+), 15 deletions(-)

-- 
2.27.0

[PATCH V2 1/4] bash-completion:fix shellcheck error

[t.feng]

SC2070 (error): -n doesn't work with unquoted arguments.
Quote or use [[ ]].
In grub-completion.bash.in line 130:
             [ -n $tmp ] && {
                  ^--^ SC2070 (error)

ref:https://github.com/koalaman/shellcheck/wiki/SC2070

Signed-off-by: "t.feng" <fengtao40@huawei.com>
---
 util/bash-completion.d/grub-completion.bash.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/bash-completion.d/grub-completion.bash.in b/util/bash-completion.d/grub-completion.bash.in
index 44bf135b9..93d143480 100644
--- a/util/bash-completion.d/grub-completion.bash.in
+++ b/util/bash-completion.d/grub-completion.bash.in
@@ -127,7 +127,7 @@ __grub_list_modules () {
     local IFS=$'\n'
     COMPREPLY=( $( compgen -f -X '!*/*.mod' -- "${grub_dir}/$cur" | {
          while read -r tmp; do
-             [ -n $tmp ] && {
+             [ -n "$tmp" ] && {
                  tmp=${tmp##*/}
                  printf '%s\n' ${tmp%.mod}
              }
-- 
2.27.0

[PATCH V2 2/4] bash-completion:fix shellcheck SC2207-Warning

[t.feng]

SC2207 (warning): Prefer mapfile or read -a to split
command output (or quote to avoid splitting).

In grub-completion.bash.in line 56:
        COMPREPLY=($(compgen -P "${2-}" -W "${1-}" -S "${4-}" --
"$cur"))
                   ^-- SC2207 (warning)

In grub-completion.bash.in line 119:
        COMPREPLY=( $(compgen \
                    ^-- SC2207 (warning)

In grub-completion.bash.in line 128:
    COMPREPLY=( $( compgen -f -X '!*/*.mod' -- "${grub_dir}/$cur" | {
                ^-- SC2207 (warning)

ref:https://github.com/koalaman/shellcheck/wiki/SC2207

Signed-off-by: "t.feng" <fengtao40@huawei.com>
---
 .../bash-completion.d/grub-completion.bash.in | 34 ++++++++++++-------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/util/bash-completion.d/grub-completion.bash.in b/util/bash-completion.d/grub-completion.bash.in
index 93d143480..4749cbc64 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
 }
@@ -116,24 +119,29 @@ __grub_list_menuentries () {
 
     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
+            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 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" ] && {
+                tmp=${tmp##*/}
+                printf '%s\n' ${tmp%.mod}
+            }
+        done
+    })
 }
 
 #
-- 
2.27.0

[PATCH V2 3/4] bash-completion:fix shellcheck SC2155-Warning

[t.feng]

In grub-completion.bash.in line 115:
    local config_file=$(__grub_dir)/grub.cfg
          ^---------^ SC2155 (warning)

In grub-completion.bash.in line 126:
    local grub_dir=$(__grub_dir)
          ^------^ SC2155 (warning)

ref:https://github.com/koalaman/shellcheck/wiki/SC2155

Signed-off-by: "t.feng" <fengtao40@huawei.com>
---
 util/bash-completion.d/grub-completion.bash.in | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/util/bash-completion.d/grub-completion.bash.in b/util/bash-completion.d/grub-completion.bash.in
index 4749cbc64..d5250c179 100644
--- a/util/bash-completion.d/grub-completion.bash.in
+++ b/util/bash-completion.d/grub-completion.bash.in
@@ -115,7 +115,8 @@ __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'
@@ -129,7 +130,8 @@ __grub_list_menuentries () {
 }
 
 __grub_list_modules () {
-    local grub_dir=$(__grub_dir)
+    local grub_dir
+    grub_dir=$(__grub_dir)
     local IFS=$'\n'
     COMPREPLY=()
     while read -r line; do
-- 
2.27.0

[PATCH V2 4/4] bash-completion:disable shellcheck SC2120-Warning

[t.feng]

In grub-completion.bash.in line 63:
__grub_get_options_from_help () {
^-- SC2120 (warning)

SC2120: the current code meets the exception and does not need to be
modified. So we disable it.

ref:https://github.com/koalaman/shellcheck/wiki/SC2120

Signed-off-by: "t.feng" <fengtao40@huawei.com>
---
 util/bash-completion.d/grub-completion.bash.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/bash-completion.d/grub-completion.bash.in b/util/bash-completion.d/grub-completion.bash.in
index d5250c179..4053fc729 100644
--- a/util/bash-completion.d/grub-completion.bash.in
+++ b/util/bash-completion.d/grub-completion.bash.in
@@ -63,6 +63,7 @@ __grubcomp () {
 
 # Function that return long options from the help of the command
 # - arg: $1 (optional) command to get the long options from
+# shellcheck disable=SC2120
 __grub_get_options_from_help () {
      local prog
 
-- 
2.27.0

Re: [PATCH V2 1/4] bash-completion:fix shellcheck error

[Daniel Kiper]

On Wed, Nov 30, 2022 at 04:30:57PM +0800, t.feng wrote:
> SC2070 (error): -n doesn't work with unquoted arguments.
> Quote or use [[ ]].
> In grub-completion.bash.in line 130:
>              [ -n $tmp ] && {
>                   ^--^ SC2070 (error)
>
> ref:https://github.com/koalaman/shellcheck/wiki/SC2070
>
> Signed-off-by: "t.feng" <fengtao40@huawei.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

Re: [PATCH V2 2/4] bash-completion:fix shellcheck SC2207-Warning

[Daniel Kiper]

On Wed, Nov 30, 2022 at 04:30:58PM +0800, t.feng wrote:
> SC2207 (warning): Prefer mapfile or read -a to split
> command output (or quote to avoid splitting).
>
> In grub-completion.bash.in line 56:
>         COMPREPLY=($(compgen -P "${2-}" -W "${1-}" -S "${4-}" --
> "$cur"))
>                    ^-- SC2207 (warning)
>
> In grub-completion.bash.in line 119:
>         COMPREPLY=( $(compgen \
>                     ^-- SC2207 (warning)
>
> In grub-completion.bash.in line 128:
>     COMPREPLY=( $( compgen -f -X '!*/*.mod' -- "${grub_dir}/$cur" | {
>                 ^-- SC2207 (warning)
>
> ref:https://github.com/koalaman/shellcheck/wiki/SC2207
>
> Signed-off-by: "t.feng" <fengtao40@huawei.com>
> ---
>  .../bash-completion.d/grub-completion.bash.in | 34 ++++++++++++-------
>  1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/util/bash-completion.d/grub-completion.bash.in b/util/bash-completion.d/grub-completion.bash.in
> index 93d143480..4749cbc64 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

I think the "line" variable should be local. Same for other variables below...

And I think it would be nice if you add a patch which marks all existing
functions variables as local.

> +            COMPREPLY+=("${line}")
> +        done < <(compgen -P "${2-}" -W "${1-}" -S "${4-}" -- "$cur")
>          ;;
>      esac
>  }
> @@ -116,24 +119,29 @@ __grub_list_menuentries () {
>
>      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
> +            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 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" ] && {
> +                tmp=${tmp##*/}
> +                printf '%s\n' ${tmp%.mod}
> +            }
> +        done
> +    })
>  }

Daniel

Re: [PATCH V2 3/4] bash-completion:fix shellcheck SC2155-Warning

[Daniel Kiper]

On Wed, Nov 30, 2022 at 04:30:59PM +0800, t.feng wrote:
> In grub-completion.bash.in line 115:
>     local config_file=$(__grub_dir)/grub.cfg
>           ^---------^ SC2155 (warning)
>
> In grub-completion.bash.in line 126:
>     local grub_dir=$(__grub_dir)
>           ^------^ SC2155 (warning)

Please add at least one sentence of description to the commit message
why this patch is needed. Otherwise almost everybody has to take a look
at the link below what the warning means. Though link may stay as is...

> ref:https://github.com/koalaman/shellcheck/wiki/SC2155

Please drop "ref:" or replace it with "More: ". Same applies to the
other patches.

> Signed-off-by: "t.feng" <fengtao40@huawei.com>

Daniel

Re: [PATCH V2 4/4] bash-completion:disable shellcheck SC2120-Warning

[Daniel Kiper]

On Wed, Nov 30, 2022 at 04:31:00PM +0800, t.feng wrote:
> In grub-completion.bash.in line 63:
> __grub_get_options_from_help () {
> ^-- SC2120 (warning)

Again, what the warning means?

> SC2120: the current code meets the exception and does not need to be
> modified. So we disable it.
>
> ref:https://github.com/koalaman/shellcheck/wiki/SC2120
>
> Signed-off-by: "t.feng" <fengtao40@huawei.com>

Daniel