8
\$\begingroup\$

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 <value>] <github_username> [<path>]"

# 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 [<value>]    <value> 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
            roundup="$(type -P 'roundup')"
            [[ ! -z $roundup ]] || {
                printf "$(tput setaf 1)⊘ Error:$(tput sgr0) %s. Aborting!\n" "Roundup is required to run tests"
                printf "\n"
                printf "Download it at https://github.com/bmizerany/roundup"
                exit 2;
            }
            $roundup ./tests/*.sh
            exit 0
            ;;
        -v | --version) # Version of software
            printf "%s\n" "Version $(git describe --tags)"
            exit 0
            ;;
        --) # End of all options
            printf "%s\n" "-bash: $(basename "$0"): $1: invalid option"
            printf "%s" "$usage"
            exit 1
            ;;
        -*)
            printf "%s\n" "-bash: $(basename "$0"): $1: invalid option"
            printf "%s" "$usage"
            exit 1
            ;;
        *)  # No more options
            break
            ;;
    esac
done

# Check if username is passed in, if not bail
if [[ -z "$1" ]]; then
    printf "$(tput setaf 1)⊘ Error:$(tput sgr0) %s. Aborting!\n" "A valid Github user is required" 
    exit 3
fi

# check if directory is not blank and exists
if [[ ! -z "$2" && -d "$2" ]]; then
    # http://www.linuxforums.org/forum/programming-scripting/solved-delete-trailing-slashes-using-bash-board-means-print-172714.html
    # This matches from the start of the source string, any 
    # string ending with a non-slash.
    pattern="^.*[^/]"
    # Apply regex
    [[ ${2} =~ $pattern ]]
    # Print the portion of the source string which matched the regex.
    path="${BASH_REMATCH[0]}"
fi

# set some variables
user="$1"
api_url="https://api.github.com/users/${user}/${feed}"
current_page=1
per_page=100

printf "%s" "Checking status of user '${user}'"
# http://stackoverflow.com/questions/238073/how-to-add-a-progress-bar-to-a-shell-script
# start progress bar
while :;do echo -n .;sleep 1;done &

# check response header from github user passed in
# http://stackoverflow.com/a/10724976/1536779
response="$(curl --write-out %{http_code} --silent --output /dev/null "${api_url}")"

# kill the progress bar
kill $!; trap 'kill $!' SIGTERM

# if reponse is greater than or equal to 400 somethings wrong
if [[ "${response}" -ge 400 ]]; then
    printf "%s\n" "-bash: $(basename "$0"): $1: user doesn't exist"
    #debug statement
    printf "%s\n" "Github HTTP Response code: ${response}"
    exit 3
fi


# grab the total number of pages there are
# https://gist.github.com/michfield/4525251
total_pages="$(curl -sI "${api_url}?page=1&per_page=${per_page}" | sed -nE "s/^Link:.*page=([0-9]+)&per_page=${per_page}>; rel=\"last\".*/\1/p")"
if [[ -z ${total_pages} ]]; then
    total_pages=1
fi

# grab a list of repos or gists
# @params $1: page number
# example: get_repos_list 1
get_repos_list() {
    # get a json list of all repos and story as array
    if [[ ${feed} != 'gists' ]]; then
        repos=$(curl -fsSL "${api_url}?page=${1}&per_page=${per_page}" | jq '.[] | .name')
    else
        repos=$(curl -fsSL "${api_url}?page=${1}&per_page=${per_page}" | jq '.[] | .id')
    fi
    echo "$repos"
}

# loop through list of repos at the current page
clone_shit() {
    printf "%s" "Grabbing list of ${feed} for ${user}"
    # http://stackoverflow.com/questions/238073/how-to-add-a-progress-bar-to-a-shell-script
    # start progress bar
    while :;do echo -n .;sleep 1;done &

    # get the list of repos for user
    repos_list=($(get_repos_list "${current_page}"))

    # kill the progress bar
    kill $!; trap 'kill $!' SIGTERM
    printf "\n"

    # loop through all repos in array
    for index in ${!repos_list[*]}
    do
        # variable assignment
        repo="${repos_list[$index]}"
        # Strip quotes from string
        repo="${repo:1:${#repo}-2}"

        if [[ ${feed} != "gists" ]]; then
            printf "%s\n" "Cloning https://github.com/${user}/${repo} to ${path}/repos/${repo}"
            #git clone "https://github.com/${user}/${repo}" "${path}/repos/${repo}"
        else
            printf "%s\n" "Cloning https://gist.github.com/${repo}.git to ${path}/gists/${repo}"
            #git clone "https://gist.github.com/${repo}.git" "${path}/gists/${repo}"
        fi
    done
}

printf "\n"

clone_shit
if [[ ${total_pages} -gt 1 && ${current_page} -lt ${total_pages} ]]; then
    current_page=$((current_page + 1))
    clone_shit
fi
\$\endgroup\$
  • \$\begingroup\$ Only glanced for a second but if [[ "$(some command)" ]]; then is very redundant when you want to depend on the commands return value. if some command; then works just fine. \$\endgroup\$ – Etan Reisner Apr 24 '14 at 22:37
  • \$\begingroup\$ @EtanReisner, would you point to a specific example? \$\endgroup\$ – iamnewton Apr 24 '14 at 22:44
  • 1
    \$\begingroup\$ if [[ ! "$(type -P 'git')" ]]; then can be replaced with if ! type -P 'git' >/dev/null; then for example. Which works because type returns true if it finds its argument and false otherwise and avoids the subshell and string comparison (against the empty string). \$\endgroup\$ – Etan Reisner Apr 24 '14 at 23:18
  • 1
    \$\begingroup\$ Your use of printf is a bit odd. You give it a format specifier of %s and then interpolate variables into its only argument and have it print that. It would be better to give it the actual string you want the variables interpolated into using its formatting specifiers and then giving it the variables as arguments. printf 'Grabbing ... %s for %s' "${feed}" "${user}" instead of printf '%s' "Grabbing .. ${feed} for ${user}". \$\endgroup\$ – Etan Reisner Apr 25 '14 at 2:03
  • 1
    \$\begingroup\$ printf '%s⊘ Error:%s Git is required to use %s. Aborting!\n' "$(tput setaf 1)" "$(tput sgr0)" "$(basename "$0")" the output from tput is just characters like anything else (not to mention that your double quote version doesn't work in an interactive shell with history expansion on because the shell tries to expand the ! and explodes. \$\endgroup\$ – Etan Reisner Apr 25 '14 at 5:02
5
\$\begingroup\$

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/#
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.