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 tidycleartput boldrandomColourif [[ $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    echo 'Wise choice, if you change your mind then run again and answer yes, or just edit this script (which is all that pressing y would have done)'    fi    exit 0fiecho '      .___                                            |   | _____ _____     ____   ____               |   |/     \\__  \   / ___\_/ __ \              |   |  Y Y  \/ __ \_/ /_/  >  ___/              |___|__|_|  (____  /\___  / \___  >                       \/     \//_____/      \/        __________              .__                     \______   \ ____   _____|__|_______ ___________  |       _// __ \ /  ___/  \___   // __ \_  __ \  |    |   \  ___/ \___ \|  |/    /\  ___/|  | \/ |____|_  /\___  >____  >__/_____ \\___  >__|           \/     \/     \/         \/    \/       'randomColourecho Enter settings, or press enter for defaults:randomColour eecho Loading directory paths from Perlsuccess=failurecd ../www/properties/lib/uploadDataStore=$(perl -e 'use WIMSConfig; $z = WIMSConfig->new(); print $z->{uploadDataStore}')if [[ $? == 0 ]] && [[ $uploadDataStore !=  ]]; thencurrentDataStore=$(perl -e 'use WIMSConfig; $z = WIMSConfig->new(); print $z->{currentDataStore}')    if [[ $? == 0 ]] && [[ $currentDataStore !=  ]]; then        success=success    fifiif [[ $success != success ]]; then    randomColour    echo Failure: Problem setting directory paths from perl code    exit 100fitput setaf 4randomColour eecho fyi, I have uploadDataStore of $uploadDataStore and currentDataStore of $currentDataStoreechosuccess=nowhile [[ $success == no ]]; dorandomColourecho 'Which directory from within uploadDataStore to get files from? (default = subfolder)'randomColourread theDirectoryif [[ $theDirectory ==  ]]; then    theDirectory=subfolderfiif [[ -e $uploadDataStore$theDirectory ]]; then    success=yeselse    randomColour    echo Can't find a folder called $uploadDataStore$theDirectory    echofidonedeclare -i NameCounterdeclare -A fileNameToNumberMapdeclare -A numberToFileNameMapNameBuilder='Submit=Save+and+Process'success=nowhile [[ $success == no ]]; dorandomColour eecho 'What is name of CSV file? (default = csvfile.csv)'randomColourread CSVFilenameif [[ $CSVFilename ==  ]]; then    CSVFilename=csvfile.csvfiCSVFilename=$uploadDataStore$theDirectory/$CSVFilenameif [[ -e $CSVFilename ]]; then    success=yeselse    randomColour    echo Counld not find file by name of $CSVFilename, try againfidonetempfile=$(mktemp)echo -n $NameBuilder > $tempfilecat $CSVFilename | php -r 'while(($a = fgets(STDIN))!==false) echo str_replace(---___---,\n,str_replace(-_.-_.,,,urlencode(str_replace(\n,---___---,str_replace(,,-_.-_.,$a)))));' | awk -F , '{prefix=sprintf(%d_,NR);printf(&%s_Name=%s&%sdataStore=%s&%sbranch=%s&%sdevelopmentName=%s&%saltDevelopmentName=%s&%slocation=%s&%scompletionYear=%s&%sdevelopmentType=%s&%sdevelopmentComprises=%s&%sstageOfCompletion=%s&%sshotType=%s&%sdetail%s&%simageType=%s&%scopyright=%s&%sphotoExists=%s&%soriginalFilename=%s,prefix,$1,prefix,$3,prefix,$4,prefix,$5,prefix,$6,prefix,$7,prefix,$8,prefix,$9,prefix,$10,prefix,$11,prefix,$12,prefix,$13,prefix,$14,prefix,$15,prefix,$16,prefix,$17)}' >> $tempfiletempfileSize=$(stat -c%s $tempfile)echo Starting Perl ScriptrandomColour ecd ../cat $tempfile | QUERY_STRING=Folder=$theDirectory&action=doProcessUploads CONTENT_LENGTH=$tempfileSize perl -T index.cgi 2>&1 | sed  -u -n '/NEW IMAGE/ {s/\(.*\)IMAGE\(.*\)/Perl looks like it is processing image\2/p}' | everyLineADifferentColourcd ../../upload/randomColour eecho 'Hope you enjoyed the ****** Image Resizer Experience!'tput setaf 0tput sgr0
Bash script - automate an upload image + description process on a server
php;bash;linux;console;awk
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 stuffO *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 hereBUTSomewhere 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 stuffIf you cut through the crap, you have an incomplete script... Lets go through some things..TrapsThis 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 tidyApart 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 togetherthere is no need for the exit inside the trapIt is more readable to use the signal numbers instead of the namesyou 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 EXITError HandlingYou have none.... and, it's really easy... do:set -enear 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  command that fails is part of the command                  list immediately following a  while  or  until  keyword,                  part  of  the  test  following  the  if or elif reserved                  words, part of any command executed in a && or  ||  list                  except  the  command  following  the final && or ||, any                  command in a pipeline but the last, or if the  command's                  return  value  is being inverted with !.  A trap on ERR,                  if set, is executed before the shell exits.  This option                  applies to the shell environment and each subshell envi                  ronment separately (see  COMMAND  EXECUTION  ENVIRONMENT                  above), and may cause subshells to exit before executing                  all the commands in the subshell.Terminal FunkinessYou clear the terminal.... and you play with terminal colour and boldness, etc.What if you are on a simple terminal that does not support these features? Your program will either fail, or will look really ugly.A user operating in a terminal is looking for efficiency and usability. The trates you have added are not efficient, and ruin the usability experience the user wants... (otherwise they would be in a GUI...)Printing out a huge banner is also not useful... what if the user has a small screen... it's ugly,randomColourecho Enter settings, or press enter for defaults:randomColour eThe above implies that you are expecting user input, but then you don't... odd.Directory RequirementsYou expect your user to be inside a specific directory, and this is not 'friendly'. It means a user cannot add the program to their $PATH and expect it to work:cd ../www/properties/lib/You can solve this problem by putting the script itself in a know directory, and then getting the actual directory name from there, and then using relative directories:SCRIPT_PATH=$(cd `dirname ${BASH_SOURCE[0]}` && pwd)Then you can use that variable like:cd $SCRIPT_PATH/../www/properties/lib/although, that is not ideal, you should not need to be changing directories in the main part of the script.User InputYou get a directory from the user.... inside a loop:while [[ $success == no ]]; doBut, your indentation seems to die inside that loop. Makes it hard to read.Also, you should have a mechanism for the stript to take the argument from the commandline, and only prompt for input if there was no commandline argument...# check first argument.     theDirectory=$1 if [[ -d $theDirectory ]] ; then    success=yesfiSame thing for the CSVFile.