Small bash script to quickly edit scripts in $PATH

Question

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 or exit $? if want to be super-explicit)

There might be other things I can improve on, so feel free to point them out.

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[@]}"