Git commit-msg URL shortener

Question

I have just written my first git hook script. It is very simple that simply finds any URLs in the commit message and uses the Google URL shortener to rewrite the URL nicely.

it is located here.

I feel it could be improved immensely (as it is my first) and would love to have your input.

#! /bin/bash
message=`cat $1`
shorten=`sed -ne 's/.*\(http[^"]*\).*/\1/p' $1`
echo "Shortening Url $shorten ...."
new_url=`curl -s https://www.googleapis.com/urlshortener/v1/url \-H 'Content-Type: application/json' \-d "{'longUrl': '$shorten'}"`
latest=`echo $new_url | python -c 'import json,sys;obj=json.load(sys.stdin);print obj["id"]';`
final=${message/$shorten/$latest}
echo $final > $1

Answer 1

First of all, the `cmd` form of running commands and capturing their output is deprecated. Use the recommended, modern way everywhere: $(cmd)

Safety

Be careful with spaces in filenames. These commands will break:

message=`cat $1`
shorten=`sed -ne 's/.*\(http[^"]*\).*/\1/p' $1`

It will be safer like this:

message=$(cat "$1")
shorten=$(sed -ne 's/.*\(http[^"]*\).*/\1/p' "$1")

The pattern there in the sed is not very safe. The text "add support for http protocol" will match. I think you want to be more strict, maybe something like:

longUrl=$(sed -ne 's/.*\(https\{0,1\}:\/\/[^"]*\).*/\1/p' "$1")

Note: if you have GNU sed (you're in Linux, or have gsed), then instead of the tedious https\{0,1\} you can simplify as https\?, though it's less portable.

And what if there are multiple URLs in the script? It will fail. You probably want to loop over the results of the sed. Or take a lazier approach and just ensure that sed will always produce at most one line:

longUrl=$(sed -ne 's/.*\(https\{0,1\}:\/\/[^"]*\).*/\1/p' "$1" | head -n 1)

What if there are no matches? An if would be good, as in that case you won't need the curl call, and it's better to not overwrite a file if you don't really have to.

When you echo $somevar, whitespaces inside like tabs and newlines would be replaced by spaces. To prevent that, quote the variable:

echo "$message" > "$1"

Naming

Your variable names are not so good:

• shorten is the output of sed, a long url. Something like longUrl would have been better.
• newUrl is the output of curl, a json text. Something like json would have been better.

Unnecessary things

In the curl command, you don't need the backslashes in \-H and \-d flags.

And instead of saving the output of the curl and then echo-ing to python, it would be better to skip that intermediary variable and just pipe it directly, like this:

shortUrl=$(curl -s https://www.googleapis.com/urlshortener/v1/url -H 'Content-Type: application/json' -d "{'longUrl': '$longUrl'}" | python -c 'import json, sys; print(json.load(sys.stdin)["id"])')

Also notice in that python that I skipped the intermediary obj variable and just used the ["id"] directly on json.load(...).

Suggested implementation

#! /bin/bash
message=$(cat "$1")
longUrl=$(sed -ne 's/.*\(https\{0,1\}:\/\/[^"]*\).*/\1/p' "$1" | head -n 1)
if test "$longUrl"; then
    echo "Shortening Url $longUrl ..."
    shortUrl=$(curl -s https://www.googleapis.com/urlshortener/v1/url -H 'Content-Type: application/json' -d "{'longUrl': '$longUrl'}" | python -c 'import json, sys; print(json.load(sys.stdin)["id"])')
    message=${message/$longUrl/$shortUrl}
    echo "$message" > "$1"
fi

This is still not perfect, because it doesn't handle multiple urls. I might add that later, gotta go now.

Online shell checker

This site is pretty awesome: http://www.shellcheck.net/#

Copy-paste your script in there, and it can spot many mistakes that are easy fix.