All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] bash-completion:fix shellcheck error and warning
@ 2022-09-19  1:20 t.feng
  2022-09-19  1:20 ` [PATCH 1/2] bash-completion:fix shellcheck error t.feng
  2022-09-19  1:20 ` [PATCH 2/2] bash-completion:fix shellcheck warning t.feng
  0 siblings, 2 replies; 6+ messages in thread
From: t.feng @ 2022-09-19  1:20 UTC (permalink / raw)
  To: grub-devel; +Cc: fengtao40, daniel.kiper, yanan, zhaowei23

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

t.feng (2):
  bash-completion:fix shellcheck error
  bash-completion:fix shellcheck warning

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

-- 
2.27.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] bash-completion:fix shellcheck error
  2022-09-19  1:20 [PATCH 0/2] bash-completion:fix shellcheck error and warning t.feng
@ 2022-09-19  1:20 ` t.feng
  2022-11-23 15:57   ` Daniel Kiper
  2022-09-19  1:20 ` [PATCH 2/2] bash-completion:fix shellcheck warning t.feng
  1 sibling, 1 reply; 6+ messages in thread
From: t.feng @ 2022-09-19  1:20 UTC (permalink / raw)
  To: grub-devel; +Cc: fengtao40, daniel.kiper, yanan, zhaowei23

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

---
 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



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] bash-completion:fix shellcheck warning
  2022-09-19  1:20 [PATCH 0/2] bash-completion:fix shellcheck error and warning t.feng
  2022-09-19  1:20 ` [PATCH 1/2] bash-completion:fix shellcheck error t.feng
@ 2022-09-19  1:20 ` t.feng
  2022-11-23 16:09   ` Daniel Kiper
  1 sibling, 1 reply; 6+ messages in thread
From: t.feng @ 2022-09-19  1:20 UTC (permalink / raw)
  To: grub-devel; +Cc: fengtao40, daniel.kiper, yanan, zhaowei23

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

---
 .../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
+            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 "] && {
+                tmp=${tmp##*/}
+                printf '%s\n' ${tmp%.mod}
+            }
+        done
+    })
 }
 
 #
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] bash-completion:fix shellcheck error
  2022-09-19  1:20 ` [PATCH 1/2] bash-completion:fix shellcheck error t.feng
@ 2022-11-23 15:57   ` Daniel Kiper
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Kiper @ 2022-11-23 15:57 UTC (permalink / raw)
  To: fengtao40; +Cc: grub-devel, yanan, zhaowei23

On Mon, Sep 19, 2022 at 09:20:13AM +0800, t.feng via Grub-devel 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

Missing SOB. Otherwise LGTM...

> ---
>  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}
>               }

Daniel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] bash-completion:fix shellcheck warning
  2022-09-19  1:20 ` [PATCH 2/2] bash-completion:fix shellcheck warning t.feng
@ 2022-11-23 16:09   ` Daniel Kiper
  2022-11-29 15:51     ` Fengtao (fengtao, Euler)
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Kiper @ 2022-11-23 16:09 UTC (permalink / raw)
  To: fengtao40; +Cc: grub-devel, yanan, zhaowei23

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...

> ---
>  .../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?

> +            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.

> +                tmp=${tmp##*/}
> +                printf '%s\n' ${tmp%.mod}
> +            }
> +        done
> +    })
>  }

Daniel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] bash-completion:fix shellcheck warning
  2022-11-23 16:09   ` Daniel Kiper
@ 2022-11-29 15:51     ` Fengtao (fengtao, Euler)
  0 siblings, 0 replies; 6+ messages in thread
From: Fengtao (fengtao, Euler) @ 2022-11-29 15:51 UTC (permalink / raw)
  To: Daniel Kiper, grub-devel; +Cc: yanan



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
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-11-29 15:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-19  1:20 [PATCH 0/2] bash-completion:fix shellcheck error and warning t.feng
2022-09-19  1:20 ` [PATCH 1/2] bash-completion:fix shellcheck error t.feng
2022-11-23 15:57   ` Daniel Kiper
2022-09-19  1:20 ` [PATCH 2/2] bash-completion:fix shellcheck warning t.feng
2022-11-23 16:09   ` Daniel Kiper
2022-11-29 15:51     ` Fengtao (fengtao, Euler)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.