Script for handling PPA's on Ubuntu

I'm writing a bash script to handle the PPA's on Ubuntu & derivatives. I was wondering how to know if I'm handling this the right way.

So, the script works (flawlessly), but I posted it to a blog and got this feedback :


The way you wrote you script is the one we all use when we begin : we don't know the variables, so we use traditionnal commands, and in the end if we remove the "#" arguments from the script, the code is sad (maybe he meant 'poor' ?)

Here is a sample :

#! /bin/bash
SCRIPT="ppa-tool"
VERSION="2.0.1" 
DATE="2014-03-26"
RELEASE="$(lsb_release -si) $(lsb_release -sr)"

[ -n "`echo "$1" | grep ppa`" ] && PPA=$1
[ -n "`echo "$2" | grep ppa`" ] && PPA=$2

helpsection() 
{ 
echo -e "Usage : $SCRIPT [OPTION]... [PPA]... 
-h, --help     shows this help
-c, --check    check if [PPA] is available for your release

Version $VERSION - $DATE"
exit 0
}

error() { echo -e "$SCRIPT - Oops, something went wrong\nTry « $SCRIPT --help » for more information." && exit 0 ; }

ppa_verification()
{ 
wget http://ppa.launchpad.net/$(echo $PPA | sed -e 's/ppa://g')/ubuntu/dists -O /tmp/"$SCRIPT-check.tmp" -q 
if [[ -n "`cat "/tmp/$SCRIPT-check.tmp" | grep $(lsb_release -sc)`" ]] ; then 
    echo -e "$SCRIPT : '$PPA' is available for $RELEASE"
else 
    echo -e "$SCRIPT : '$PPA' is NOT available for $RELEASE"
fi  
rm "/tmp/$SCRIPT-check.tmp"
}

[ "$1" == "--help" ] || [ "$1" == "-h" ] && helpsection

if [ "$1" == "--check" ] || [ "$1" == "-c" ] || [ "$1" == "check" ]  ; then
    [ -n "`echo $2`" ] && ppa_verification && exit 0
    error
fi

error

(complete script - also in French)

Apparently, my syntax is bad, but I don't see what I could do to improve this, and I don't understand the feedback I recieved. Could you help me figure it out ?

EDIT: From Unix&Linux (where I first posted, but was redirected here), someone said it wasn't good to use capital letters for variables. So $SCRIPT should become $script.

User experience

• -h or --help should print the usage message to standard output, which you do. However, on any unrecognizable command line, you should print the same (or similar) message to standard error and exit with a non-zero status.

• Similarly, other error messages should go to standard error and result in a non-zero status.

Style issues

• Good job, double-quoting almost every variable you used. A lot of shell programmers neglect to do so.

• Good job, breaking the program up into functions. A lot of shell programmers don't.

• Indent your code consistently. It's hard to see where your functions begin and end.

• Make use of case to replace convoluted if statements.

• Do all of your command-line parsing in one place, and have a coherent parsing strategy. You can either use getopt (which has a bit of a learning curve) or build something based on shift (which is easier for a beginner to understand).

• You can pass parameters to functions. Passing $PPA to ppa_verification() would be more elegant than using a global.

There isn't 100% consensus on whether variable names should be $ALL_CAPS or $lower_case. One common convention is $ALL_CAPS for global and/or exported variables, $lower_case for local or non-exported variables.

Use of wget

• Instead of writing the output of wget to a temporary file, you could just pipe it to grep directly: wget -q -O - "$url" | grep -q "$release".

• But then, to distinguish between a wget failure (e.g. network problem) and the release not being found by grep, you would have to interrogate ${PIPESTATUS[0]} and ${PIPESTATUS[1]}, which is a bit complicated.

• Why not try to fetch the subdirectory of dists/ that you care about instead? Then you only need to test whether the server returned an HTTP 200 (Success!) or HTTP 404 (Not Found).

#! /bin/bash
SCRIPT="ppa-tool"
VERSION="200_success"
DATE="2014-03-26"
RELEASE="$(lsb_release -si) $(lsb_release -sr)"

helpsection() 
{ 
    echo "Usage : $SCRIPT [OPTION]... [PPA]... 
-h, --help     shows this help
-c, --check    check if [PPA] is available for your release

Version $VERSION - $DATE"
}

ppa_verification()
{ 
    local ppa="${1#ppa:}"

    local codename="$(lsb_release -sc)"
    local url="http://ppa.launchpad.net/$ppa/ubuntu/dists/$codename/"

    wget "$url" -q -O /dev/null
    ######################################################################
    # Exit Status
    #
    # Wget may return one of several error codes if it encounters problems.
    # 0 No problems occurred.
    # 1 Generic error code.
    # 2 Parse error--for instance, when parsing command-line options, the `.wgetrc' or `.netrc'...
    # 3 File I/O error.
    # 4 Network failure.
    # 5 SSL verification failure.
    # 6 Username/password authentication failure.
    # 7 Protocol errors.
    # 8 Server issued an error response.
    ######################################################################
    case $? in
      0) # Success
        echo "$SCRIPT : '$ppa' is available for $RELEASE"
        ;;
      8) # HTTP 404 (Not Found) would result in wget returning 8
        echo "$SCRIPT : '$ppa' is NOT available for $RELEASE"
        return 1
        ;;
      *)
        echo "$SCRIPT : Error fetching $url" >&2
        return 3
    esac
}

PPA=
while [ -n "$*" ] ; do
    case "$1" in
      -h|--help)
        helpsection
        exit 0
        ;;
      --check=*)
        PPA="${1#*=}"
        ;;
      -c|--check|check)
        PPA="$2"
        shift
        ;;
      *)
        helpsection >&2
        exit 2
        ;;
    esac
    shift
done

if [ -z "$PPA" ]; then
    helpsection >&2
    exit 2
fi

ppa_verification "$PPA"

StackExchange Code Review Q#45445, answer score: 7