Bash script - automate an upload image + description process on a server

I was tasked at work to improve a system where through a web interface a client, which eventually became us, uploads a bunch of images with descriptions (latter coming from csv files), the images are then automatically resized yada yada for the site.

Nowt wrong with the latter, but the upload and image description data entry is unbelievably tedious - the latter having to be filled out by hand and the uploads done through http.

The usual way to do this would be for me to create a more sensible interface. However, since it's us that end up doing it and me being inexperienced at *nix sysadmin type stuff and wanting to gain some experience, I thought it would be more interesting to write a shell script to do it accessed by SSH, on the basis we can upload the CSVs and image files easily enough. Have also used sed, perl, and a line of php to help me encode data as it comes from the CSV.

With the exception of removing my company name from the logo, here is the unaltered code:

How could this shell script be improved?

```
#! /bin/bash

# Yes, the below is ludicrous overkill.

# B
# U
# T

# I need to become good enough at shell scripting that I don't have to look up
# elementary syntax before I write anything that is not just a collection of
# commands.

# It was fun :)

function quitTheBestShellScriptInTheWorld {
tput setaf 0
tput sgr0
if [[ $tempFile != "" ]]; then
rm $tempFile
fi
exit
}

function randomColour {
tput setaf $[ RANDOM % 8]
if [[ $1 == "e" ]]; then
echo
fi
}

function everyLineADifferentColour {
while read eachLine
do
randomColour
echo $eachLine
done

}

trap quitTheBestShellScriptInTheWorld 0 1 2 3 15 #let's be tidy

clear

tput bold
randomColour

if [[ $1 == "-l" ]]; then
echo "Er, logging feature not actually programmed. Add it yourself? Will be reasonably straightforward [y/n]"
read -n 1 dolog

if [[ $dolog == "y" ]]; then
(nano coolscript.sh)
else

Your code is so full of irrelevant content that distracts from the purpose of the code.

The most Beautiful shell scripts are the ones that do a job, and do it well. They do it in a standard way with the least surprise. They accept input from pipes, redirects, and files, and they output in a way that is redirectable, loggable, and readable.

The following is my long-winded-way of reviewing the superfluous fluff that makes what could otherwise be a Beautiful script (there is more serious content after that...)

Meant to be in Jest stuff

O V E R K I L L

_________ __
\_ ___ \ ____ _____ _____ ____ _____/ |_ ______
/ \ \/ / _ \ / \ / \_/ __ \ / \ __\/ ___/
\ \___( ) Y Y \ Y Y \ ___/| | \ | \___ \
\______ /\____/|__|_| /__|_| /\___ >___| /__| /____ >
\/ \/ \/ \/ \/ \/
.__ .__ .___ ___.
_____| |__ ____ __ __| | __| _/ \_ |__ ____
/ ___/ | \ / _ \| | \ | / __ | | __ \_/ __ \
\___ \| Y ( ) | / |__/ /_/ | | \_\ \ ___/
/____ >___| /\____/|____/|____/\____ | |___ /\___ >
\/ \/ \/ \/ \/
.___ ___. .__ .___
_______ ____ _____ __| _/____ \_ |__ | | ____ _____ ____ __| _/
\_ __ \_/ __ \\__ \ / __ |\__ \ | __ \| | _/ __ \ \__ \ / \ / __ |
| | \/\ ___/ / __ \_/ /_/ | / __ \| \_\ \ |_\ ___/ / __ \| | \/ /_/ |
|__| \___ >____ /\____ |(____ /___ /____/\___ > (____ /___| /\____ |
\/ \/ \/ \/ \/ \/ \/ \/ \/
_____ .__ ._._._.
__ __ ______ _____/ ____\_ __| || | | |
| | \/ ___// __ \ __\ | \ || | | |
| | /\___ \\ ___/| | | | / |_\|\|\|
|____//____ >\___ >__| |____/|____/_____
\/ \/ \/\/\/


Gimmicks are fun, but, they get tiring fast.

And, they break the 'spirit' of UNIX scripting, which is that they can do things for you in as many situations as possible.

so, what if you wanted to use this in a pipe?

what if you are running this from a screen terminal?

What if you are using a small mobile device to access things?

What if the clutter simply overwhelms the rest of it, and the point of your program is lost.

That is what has happened here

• B

• U

• T

Somewhere 1n th3r3 1s A re4l pr0gr4m, although... I was convinced at:

quitTheBestShellScriptInTheWorld.

Who am I to be critical?

At least you spelled colour correctly ;-)

Serious stuff

If you cut through the crap, you have an incomplete script... Lets go through some things..

Traps

This is your trap:

function quitTheBestShellScriptInTheWorld {
    tput setaf 0
    tput sgr0
    if [[ $tempFile != "" ]]; then
        rm $tempFile
    fi
    exit
}

And you enable 20 lines later with:

trap quitTheBestShellScriptInTheWorld 0 1 2 3 15 #let's be tidy

Apart from the humble function name, the system suffers because:

• you should set the trap in the same code-location as the method so that it is obvious they belong together

• there is no need for the exit inside the trap

• It is more readable to use the signal numbers instead of the names

• you only need to trap EXIT (which will be invoked whenever the shell exits (successful or not)

• you should set a the tempFile variable so it has a clear value before the trap is set.

It should look like:

# Setting up the exit Clean-Up trap.

tempfile=""

function exiting {
    if [[ -f $tempFile ]]; then
        rm $tempFile
    fi
}

trap exiting EXIT

Error Handling

You have none.... and, it's really easy... do:

set -e

near the top of your script.

If any command fails then your program will terminate (with a non-zero exit code).

If you have commands that you expect to fail, you can test them with if statements, or put conditionals after them so that the status is handled....

```
set -e Exit immediately if a pipeline (which may consist of a
single simple command), a subshell command enclosed in
parentheses, or one of the commands executed as part of
a command list enclosed by braces (see SHELL GRAMMAR
above) exits with a non-zero status. The shell does not
exit if the

StackExchange Code Review Q#42432, answer score: 12