Is this shell script for copying files sensible, readable?

As an exercise in learning bash, I wrote this script designed to automate the process of populating a drive with uncompressed .aiff files copied directly from a CD. It saves me having to do a bunch of separate commands to create subdirectories, check before clobbering, etc.

It stores them in this fashion:

Root directory -> Artist -> Album -> Disc in the series (if necessary)

I wrote it to be run in bash on a Mac. Hence the "open" command.

-
What best practices am I ignoring?

-
Is any of the logic unnecessarily complex? (Aside from the whole endeavor, which I suppose I could just do through the GUI... but where's the fun in that?)

Specifically, I wonder about comparing the contents of an origin directory to a destination directory by outputting the destination's content to a text file and running an inverse grep search on it:

for trackname in $(ls -1 /Volumes/$the_disc | grep -vf ~/.harvest/existing); do

Thanks for any feedback you can offer this newbie.

```
#!/bin/bash

# > harvest.sh

vrs='v 0.7'
lastchange='8/15/11'

echo -e "\n--> Harvest $vrs $lastchange I can't find any more external volumes. Are you sure you inserted a CD or external drive?\n"

function y_or_n () {
until [ "$pick" == "y" -o "$pick" == "n" ]; do
echo -e "--> (Enter \"y\" or \"n\".)\n"
read pick
done
}

function collect () {
the_disc=$1
multi=null
ls /Volumes/$1
echo -e "\n--> What's the artist's name?\n"
read artist
echo -e "\n--> OK. We're storing music from $artist."

echo -e "\n--> Is \"$1\" the name of the album?"
pick=null
y_or_n

if [ "$pick" == "y" ]; then
echo -e "\n--> Excellent. $1 is the name of the album by $artist.\n"
album=$1
else
echo -e "\n--> What's the name of the album?\n"
read album
echo -e "\n--> Excellent. $album is the name of the album by $artist."
fi

echo -e "\n--> Is \"$album\" a multi-disc set?"
pick=null
y_or_n

if [ "$pick" == "y" ]; then
echo -e "\n--> Which disc is this? (\"1

Some comments:

• You script does not work if you have spaces in the filenames, but I guess this cannot happen.

• You do not need to restore IFS as the script will be run in a subprocess, and will not impact parent environment.

• You could run ls -1 /Volumes | grep -vf ~/.int_vols only once, doing a filenames=$(ls -1 /Volumes | grep -vf ~/.int_vols) before the if.

• You could save the -d tests in the for loop by using find /Volumes -mindepth 1 -maxdepth 1 -type d | cut -d "/" -f 3 instead of ls -1 /Volumes.

• You could save the entire for loop doing a find /Volumes -mindepth 1 -maxdepth 1 -type d | cut -d "/" -f 3 | paste -s -d " " | read -a f

The cut -d "/" -f 3 does not work well if you change /Volumes by a directory that is not directly under /. I would use sed -e s|.*/|| instead, but this introduces a new tool that you might not be familiar with.

For the rest of the script, I do not know the structure of the data, so I cannot tell.

StackExchange Code Review Q#4131, answer score: 4