Bash script to clone all public repos or gists for a specified user, optionally to a directory of choice

I wrote this script as a way to backup and/or download quickly a set of repos or gists from a user. I don't have any major concerns but I'm a noob at bash scripting, and I've been scouring the internet putting these pieces together. I would love for someone to take a look and let me know best practices or problems there may be.

```
#!/usr/bin/env bash

# Check if git is installed, if not bail
if [[ ! "$(type -P 'git')" ]]; then
printf "$(tput setaf 1)⊘ Error:$(tput sgr0) %s. Aborting!\n" "Git is required to use $(basename "$0")"
printf "\n"
printf "Download it at http://git-scm.com"
exit 2
fi

# Check if jq is installed, if not bail
if [[ ! "$(type -P 'jq')" ]]; then
printf "$(tput setaf 1)⊘ Error:$(tput sgr0) %s. Aborting!\n" "jq is required to parse JSON."
printf "\n"
printf "Download it at http://stedolan.github.io/jq"
exit 2
fi

# variables
feed="repos"
path="${HOME}/Downloads"
usage="$(basename "$0"): usage: $(basename "$0") [-h|--help] [-v|--version] [-f|--feed ] []"

# Test for known flags
for opt in "$@"
do
case "$opt" in
-f | --feed) # choose feed type
if [[ "$2" == "repos" || "$2" == "gists" ]]; then
feed="$2"
else
printf "%s\n" "-bash: $(basename "$0"): $1: invalid feed type [repos|gists]"
printf "%s" "$usage"
exit 1
fi
shift 2
;;
-h | --help) # Help text
printf "\n"
printf "%s\n" "Options:"
printf "\n"
printf "\t%s\n" "-h, --help Print this help text"
printf "\t%s\n" "-f, --feed [] can be either gists or repos, default is repos"
printf "\t%s\n" "-v, --version Print out the version"
printf "\n"
printf "%s\n" "Documentation can be found at https://github.com/chriopedia/clone-all"
exit 0
;;
--test) # test suite using roundup

It's not a bad attempt, though full of duplicated code and some bad practices.

Checking if a program exists

You do this kind of thing several times:

# Check if git is installed, if not bail
if [[ ! "$(type -P 'git')" ]]; then
    printf "$(tput setaf 1)⊘ Error:$(tput sgr0) %s. Aborting!\n" "Git is required to use $(basename "$0")"
    printf "\n"
    printf "Download it at http://git-scm.com"
    exit 2
fi

First of all, the if condition can be simplified to this:

if ! type -P git >/dev/null; then

If git is not on $PATH, the type command will fail, making the condition evaluate to true and execute the then block. This is a lot more efficient than doing a process substitution with $(...) followed by a string evaluation. I also removed the quotes from 'git', you don't need to escape bare words like "git", "jq", "round". So the same goes for those.

However, as you notice now you have to redirect the output of the type command to /dev/null. Before, this output was captured by the $(...) process substitution. Since we're not using that anymore, we need to get rid of it ourselves. It's still much cleaner this way.

Another thing in this method, you are using a second printf just to print a blank line:

printf "$(tput setaf 1)⊘ Error:$(tput sgr0) %s. Aborting!\n" "Git is required to use $(basename "$0")"
printf "\n"

It would be better to make that \n part of the first printf:

printf "$(tput setaf 1)⊘ Error:$(tput sgr0) %s. Aborting!\n\n" "Git is required to use $(basename "$0")"

Reducing duplication

You have a lot of duplication throughout. For example $(basename "$0") appears almost 10 times. It would be better to save it in a constant early in the script, for example:

me=$(basename "$0")

Error reporting also happens way too often, and since you're doing it in a fancy way, it's kind of complicated, so you probably copy-pasted it every time. Copy-pasting is never good, because if you need to change something later, you have to synchronize your changes. It would be better to create a helper method for reporting errors:

print_error() {
    printf "$(tput setaf 1)⊘ Error:$(tput sgr0) %s. Aborting!\n\n" "$1"
}

You don't need to copy-paste this fancy printf anymore, you can just do:

print_error "jq is required to parse JSON."

Similarly, you check if git, jq, roundup exist, and you always use the same pattern for this logic. This is another ideal candidate for a helper function:

require_prog() {
    prog=$1
    msg=$2
    url=$3
    type -P "$prog" >/dev/null || {
        print_error "$msg"
        echo "Download it at $url"
        exit 2
    }
}

Now your checks become simply:

require_prog git "Git is required to use $me" http://git-scm.com
require_prog jq "jq is required to parse JSON." http://stedolan.github.io/jq

Looping over $@

A nice trick that instead of for i in "$@"; do ...; done, you can shorten as:

for i; do ...; done

Using case instead of multiple OR conditions in [[ ... ]]

Instead of this:

if [[ "$2" == "repos" || "$2" == "gists" ]]; then
    feed="$2"
else
    printf "%s\n" "-bash: $(basename "$0"): $1: invalid feed type [repos|gists]"
    printf "%s" "$usage"
    exit 1
fi
shift 2

I find this way neater:

case "$2" in
    repos | gists) feed="$2" ;;
    *)
        echo "-bash: $me: $1: invalid feed type [repos|gists]"
        echo "$usage"
        exit 1
esac
shift 2

They are both fine. However, if you prefer to use [[ ... ]] then note that you don't need all those quotes you have there, you could write like this:

if [[ $2 == repos || $2 == gists ]]; then

Maybe you're thinking it's better to be safe than sorry with quotes, but I think it's better to understand how they work. Quotes are very "noisy", I find it a lot easier to read without quotes.

Tedious printf

This is really tedious:

printf "\n"
printf "%s\n" "Options:"
printf "\n"

You could do the same thing a lot easier with simple echo statements:

echo
echo Options:
echo

The above are just examples...

Your script is quite long, and full of the kind of mistakes I pointed out above. Apply the same logic as above everywhere, and your script will become cleaner and better.

shellcheck.net

There is this awesome site that can spot many common shell scripting errors and bugs:

http://www.shellcheck.net/#

StackExchange Code Review Q#48050, answer score: 5