Return to Answer

• 113k
• 15
• 154
• 396

• 113k
• 15
• 154
• 396

Use the exit code of commands instead of their output when possible

This command checks if grep's output is empty:

if [ -z "$(grep low $lowinfo)" ]; then

In addition, note that if ever `"$(grep ...)" has large output, it would be a waste of memory to store it in a string.

It's better to take advantage that grep exits with success (0) when it finds anything, and with error (anything but 0) when it doesn't find a match:

if ! grep -q low "$lowinfo"; then

An additional benefit of -q is that grep can stop processing the input immediately when it finds a match, since the output would be discarded anyway.

Double-quote variables in commands

In most places the variables are correctly double-quoted, with few exceptions, such as:

if [ -z "$(grep low $lowinfo)" ]; then
...
notify-send -i $HOME/.config/icons/critical-battery.png -u critical "Battery critically low!" 
echo "critically-low" > $lowinfo

To make these safe for special characters such as whitespace, add the missing double-quotes:

if [ -z "$(grep low "$lowinfo")" ]; then
...
notify-send -i "$HOME/.config/icons/critical-battery.png" -u critical "Battery critically low!" 
echo "critically-low" > "$lowinfo"

Prefer one statement per line

This if-else chain is a bit difficult to read because the lines contain two statements, a conditional and an action:

if [ "$capacity" -lt 15 ]; then capacity_icon=' ' 
elif [ "$capacity" -lt 40 ]; then capacity_icon=' '
elif [ "$capacity" -lt 60 ]; then capacity_icon=' ' 
elif [ "$capacity" -lt 90 ]; then capacity_icon=' '
else capacity_icon=' ' 
fi

It's easier to read with one statement per line:

if [ "$capacity" -lt 15 ]; then
 capacity_icon=' ' 
elif [ "$capacity" -lt 40 ]; then
 capacity_icon=' '
elif [ "$capacity" -lt 60 ]; then
 capacity_icon=' ' 
elif [ "$capacity" -lt 90 ]; then
 capacity_icon=' '
else
 capacity_icon=' ' 
fi

This example is the hardest to read, because readers must scroll horizontally:

([ "$capacity" -gt 20 ] || [ "$status" = "Charging" ]) && [ -n $(cat "$lowinfo") ] && echo "" > $lowinfo

With a classic if statement, and some other improvements:

if [ "$capacity" -gt 20 ] || [ "$status" = "Charging" ]; then
 [ -s "$lowinfo" ] && > "$lowinfo"
fi

The other improvements:

• Instead of writing a blank line to $lowinfo with echo "" > $lowinfo, we make it empty with > "$lowinfo".
  • I also added the missed double-quotes
• Instead of checking that the file is empty by reading its content with -n $(cat "$lowinfo"), we can use -s "..." to the same effect.
  • Notice that in case of large files, this can make a significant difference in memory use.

Use echo instead of printf when it's good enough

Instead of this:

printf "%s%s%d%%\n" "$charging_icon" "$capacity_icon" "$capacity"

I think this is easier to read with the variables embedded in the line, and also shorter:

echo "${charging_icon}${capacity_icon}${capacity}%"