Skip to content

Commit

Permalink
Do not nest quotes
Browse files Browse the repository at this point in the history 
Signed-off-by: Christian König <ckoenig@posteo.de>
  • Loading branch information
@yubiuser
yubiuser committed Jul 4, 2022
1 parent 067fac8 commit 20de1df
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion padd.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ GetVersionInformation() {
if [ "${padd_version_latest}" = "" ]; then
padd_version_heatmap=${yellow_text}
else
if [ "$(VersionConverter "${padd_version}")" -lt "$(VersionConverter "${padd_version_latest}")" ]; then
if [ "$(VersionConverter ${padd_version})" -lt "$(VersionConverter "${padd_version_latest}")" ]; then

This comment has been minimized.

Sign in to view

Copy link
@MichaIng

MichaIng Aug 4, 2022

Actually such nested quotes are correct/reasonable inside of command substitutions, now the nested command is prune to word spitting and globbing. shellcheck should also complain about missing quoting now.

This comment has been minimized.

Sign in to view

Copy link
@yubiuser

yubiuser Aug 4, 2022

Author Member

Strangely, it's not complaining...

This comment has been minimized.

Sign in to view

Copy link
@MichaIng

MichaIng Aug 4, 2022 •
edited

Ah, the variable is hardcoded (without spaces or special characters), in this case shellckeck only warns when enabling the related optional check. Safe then to keep it like it is, while best practice is to consequently double quote inside command substitutions 🙂.

This comment has been minimized.

Sign in to view

Copy link
@yubiuser

yubiuser Aug 4, 2022

Author Member

In the end we solved it like this e7dab3f
padd_out_of_date_flag="true"
padd_version_heatmap=${red_text}
else
Expand All @@ -555,6 +555,7 @@ GetVersionInformation() {
fi
fi


# was any portion of Pi-hole out-of-date?
# yes, pi-hole is out of date
if [ "${out_of_date_flag}" = "true" ]; then
Expand Down

0 comments on commit 20de1df

Please sign in to comment.