Staying true to the mantra: "The third time you do something, you automate", I've built up a decent collection of shell scripts that live in ~/bin
. Some of these scripts contain useful boilerplate code, or just stuff I can't bring myself to memorise (like colour output stuff). Other scripts I find myself elaborating on from time to time. Though not a great pain, I am very lazy. Rather than typing vim ~/bin/some_script
, I've written yet another script that expands some_script
to its full path (using which
), and opens it in vim. I'm perfectly happy with how the script works, and added a few basic creature-comforts, like checking if any (valid) arguments were provided and so on. Truth be told, I've not been writing that much bash in recent years, and some of the expressions/tricks I found myself using feel clunky. I know I used to have something like this in my .profile
, but that particular incarnation of the script/function never made it to my github misc repo.
In short, I'd like to know if there's some stuff I could improve on. First the code, then some specific things that just don't feel right to me.
Usage() {
cat <<-__EOF_
${0##*/} finds executables in PATH and opens them in vim - useful to quickly edit your scripts
-o : opens all scripts using o flag (equivalent to vim -o <paths>)
-O : similar to -o, but uses -O flag (so vim -O <paths>)
-h : Display help message
__EOF_
exit "${1:-0}"
}
flags=""
paths=""
while getopts :oOh f; do
case $f in
o)
flags="-o"
;;
O)
flags="-O"
;;
h)
Usage
;;
*)
echo "Unknown flag ${f}${OPTARG}"
Usage 1
;;
esac
shift
done
[ $# -lt 1 ] && echo "No arguments given" && Usage 2
for b in "$@"; do
p=$(which "${b}") && paths="${paths} ${p}"
done
[ ${#paths} -gt 0 ] || :(){ echo "No valid executables found in path" && exit 3; };:
# We want globbing and word splitting here
# shellcheck disable=SC2086
vim $flags $paths
Things that just feel a tad off would be:
[ $# -lt 1 ] && echo "No arguments given" && Usage 2
I do this all the time, and it works just fine, but chaining together 3 commands with&&
like this just looks ugly. If there's a cleaner, more concise way to do the same thing, I'm all ears.p=$(which "${b}") && paths="${paths} ${p}"
. I'm not sold on the use of the subshell here. I'm using#!/usr/bin/env bash
for my hashbang, I'm not sure I need the subshell. If not, what would be the alternative? What are the pro's, what are the con's?[ ${#paths} -gt 0 ] || :(){ echo "No valid executables found in path" && exit 3; };:
same as chaining things, only this time, because I'm using an||
, then an&&
, I just wrap the last 2 expressions in a function and call it. This smells IMO. I could change it to[ ${#paths} -le 0 ] && ...
, but then it's just another chain. Personally, I prefer-gt
over-le
or-eq
in this case. I also prefer to have the eventual command (vim
) to be the last one, so that the exit code of vim is passed back on success (without needing an explicit exit like[ ${#paths} -gt 0 ] && vim $flags $paths && exit
orexit $?
if want to be super-explicit)
There might be other things I can improve on, so feel free to point them out.
1 Answer 1
(削除) Use a shebang
#!/usr/bin/env bash
(削除ここまで) I see you're already doing that.
In bash, prefer [[...]]
over [...]
. The double bracket conditional
command
gives you more features (including regular expression matching), and fewer
surprises (particularly about unquoted variables).
With a getopts
loop, I prefer to shift
at the end:
while getopts ...; do
case $opt in
...
esac
done
shift $((OPTIND - 1))
Allow the Usage function to take an error message:
Usage() {
[[ -n 2ドル ]] && echo "2ドル" >&2
# ... rest is the same
}
That simplifies some assertions
(( $# > 0 )) || Usage 2 "No arguments given"
Use command -v
instead of which
Use an array to hold the paths. There's no other good way to hold a list of things where any of them might contain whitespace.
paths=()
for b in "$@"; do
p=$( command -v "$b" ) && paths+=("$p")
done
You do need to command substitution (aka subshell) so you can capture the output.
The "colon" function makes me cringe.
For clarity, use if
instead of long boolean chains
if (( ${#paths[@]} == 0 )); then
echo "No valid executables found in PATH" >&2
exit 3
fi
And prefer quoting over word splitting:
vim $flags "${paths[@]}"
Yes, the script will exit with vim's exit status. If you want to be super tidy, you could replace the script process with vim:
exec vim $flags "${paths[@]}"
-
\$\begingroup\$ Thanks for the input. The colon function felt dirty to write indeed. Just out of curiosity: any specific reason to use
(())
for numeric comparisons over[ -eq ]
& co? I get why one would prefer using>
of-gt
, but oddly enough, I find myself making less silly mistakes using the latter (I probably mess up<
vs>
at least once a week - embarrassing, I know) \$\endgroup\$Elias Van Ootegem– Elias Van Ootegem2021年03月25日 17:55:18 +00:00Commented Mar 25, 2021 at 17:55 -
\$\begingroup\$ Just a personal preference.
((...))
allow you to assign variables within it , and also to refer to variables without$
to create readable C-like expressions --((sum += hours * hourly_rate))
\$\endgroup\$glenn jackman– glenn jackman2021年03月25日 18:34:12 +00:00Commented Mar 25, 2021 at 18:34 -
\$\begingroup\$
((...))
does not play well withset -e
though: if the arithmetic expression evaluates to zero, the return status is 1 -- makes it good for conditions, but can be confusing source of errors. \$\endgroup\$glenn jackman– glenn jackman2021年03月25日 18:36:55 +00:00Commented Mar 25, 2021 at 18:36 -
\$\begingroup\$ cheers. In that case, I'll stick to the square brackets for bash scripts. Implemented pretty much all of the other suggestions you made. \$\endgroup\$Elias Van Ootegem– Elias Van Ootegem2021年03月25日 19:04:43 +00:00Commented Mar 25, 2021 at 19:04