Return to Revisions

Consider which shell to use

You've written this as a Bash script, but the only thing preventing this being a portable shell (/bin/sh) script is the use of - in the function name. Replace that with _, and we can change the shebang.

The benefits of being able to use a different shell include faster start-up, lower memory overhead and portability to systems that don't have Bash installed.

"Lint" your script

Shellcheck complains about:

280513.sh:10:34: note: Use ! grep -q instead of comparing output with [ -z .. ]. [SC2143]
280513.sh:10:60: note: Double quote to prevent globbing and word splitting. [SC2086]
280513.sh:11:20: note: Double quote to prevent globbing and word splitting. [SC2086]
280513.sh:12:29: note: Double quote to prevent globbing and word splitting. [SC2086]
280513.sh:15:36: note: Use ! grep -q instead of comparing output with [ -z .. ]. [SC2143]
280513.sh:15:51: note: Double quote to prevent globbing and word splitting. [SC2086]
280513.sh:16:20: note: Double quote to prevent globbing and word splitting. [SC2086]
280513.sh:17:18: note: Double quote to prevent globbing and word splitting. [SC2086]
280513.sh:27:30: note: Double quote to prevent globbing and word splitting. [SC2086]
280513.sh:43:1: note: Use { ..; } instead of (..) to avoid subshell overhead. [SC2235]
280513.sh:43:64: error: -n doesn't work with unquoted arguments. Quote or use [[ ]]. [SC2070]
280513.sh:43:64: warning: Quote this to prevent word splitting. [SC2046]
280513.sh:43:97: note: Double quote to prevent globbing and word splitting. [SC2086]

Minimal changes to fix these:

#!/bin/sh
# Send a notification if battery is low, change status in info file accordingly
notify_low () {
 # Battery isn't much but is charging (don't notify)
 [ "$status" = "Charging" ] && return
 # Battery critically low
 if [ "$capacity" -le 10 ] && ! grep -q critically-low "$lowinfo"; then
 notify-send -i "$HOME/.config/icons/critical-battery.png" -u critical "Battery critically low!" 
 echo critically-low >"$lowinfo"
 # Battery is low 
 elif [ "$capacity" -le 20 ] && ! grep -q low "$lowinfo"; then
 notify-send -i "$HOME/.config/icons/low-battery.png" -t 5500 "Battery low!"
 echo low >"$lowinfo"
 fi
}
battery="/sys/class/power_supply/BAT0"
status="$(cat "$battery/status")"
capacity="$(cat "$battery/capacity")"
# Get the low battery status from file ~/.cache/battery-low-status
lowinfo="$HOME/.cache/battery-low-status"
[ ! -e "$lowinfo" ] && touch "$lowinfo"
# Set charging icon and capacity icon
[ "$status" = "Charging" ] && charging_icon=" " || charging_icon=""
[ "$capacity" -gt 100 ] && capacity=100
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
# Report low battery 
[ "$capacity" -le 20 ] && notify_low
# Reset low battery information in these cases
{ [ "$capacity" -gt 20 ] || [ "$status" = "Charging" ]; } && [ -n "$(cat "$lowinfo")" ] && echo "" >"$lowinfo"
printf "%s%s%d%%\n" "$charging_icon" "$capacity_icon" "$capacity"

Minor improvements

Some things I didn't spot in the other answers:

• Consider set -u to catch misspellings of variable names, and set -e to avoid blundering on when a command fails.
• No need to read a file to check it's non-empty; replace [ -n "$(cat "$lowinfo")" ] with [ -s "$lowinfo" ].
• Prefer to pass data to functions as arguments, rather than in shared variables. In particular, I think status and capacity should be arguments to notify_low; I'm happy for lowinfo to be shared as it acts as a global constant.
• Use functions to help isolate independent sections of code. I'd have a function to select the capacity icon, even though it's used only once.
• An empty string as the only argument to echo can be removed - both echo "" and echo produce a single newline.
• Avoid testing a negative when the alternative is simpler - e.g. [ -e "$lowinfo" ] || touch "$lowinfo" instead of [ ! -e "$lowinfo" ] && touch "$lowinfo" (that said, in this case I don't see any reason not to just touch unconditionally, as we're not using the timestamp for anything).
• Prefer single-quotes when there are no variables to expand. This makes life easier for the reader, and saves the shell a tiny bit of checking, too.
• Think about preparing your user messages for translation.