I have this shellcheck
warning I can't figure out:
In /mnt/e/bin/iconic line 540:
printf "FALSE|" >> "$IconsRaw" # Select field number 1
^-- SC2129: Consider using { cmd1; cmd2; } >> file instead of individual redirects.
I've noticed many of us here use shellcheck to fix our bash scripts / shell commands so I hope the question is on topic.
As per comments posting relevant section of bash script:
if [[ "$X" == "?" || "$Y" == "?" ]] ; then
: # Bad X or Y offset usually "Link to Name.ext~" (backup name)
else
let i++
printf "FALSE|" >> "$IconsRaw" # Select field number 1
printf "%s|" "$i" >> "$IconsRaw" # 2
printf "%s|" "${File##*/}" >> "$IconsRaw"
printf "%s|" "$Linkless" >> "$IconsRaw" # 4
printf "%s|" "$Date" >> "$IconsRaw" # 5
printf "%s|" "$X" >> "$IconsRaw" # 6
echo "$Y" >> "$IconsRaw" # 7
fi
Solution
Thanks to accepted answer and comments I've learned that shellcheck
not only catches errors in your code, but also suggests performance improvements. In this case the filename $IconsRaw
was being opened and closed many times with each printf
and echo
.
The more efficient bash code:
# X,Y screen coordinates invalid on backup files ending with "~"
! [[ "$X" == "?" || "$Y" == "?" ]] && { let i++; echo \
"FALSE|$i|${File##*/}|$Linkless|$Date|$X|$Y" >> "$IconsRaw"; }
I assume your script has multiple instances of
>> "$IconsRaw"
. That message is suggesting redirecting the output only once and grouping the commands in a subshell. Presumably to avoid the overhead of opening and closing the file multiple times.So, instead of this:
This:
Bu that's also a needless repetition of
printf
and it is more efficient to just do: