Script to group files by extension and unzip archives

I am writing a shell script to sort my files by extension and unzip archives. It works in simple cases, but I haven't tested it extensively. How can I make it more robust against the set of file names that may be present?

#!/bin/bash
#File sorter
SECONDS=0
SAVEIFS=$IFS
IFS=$(echo -en "\n\b")
echo "trigger took place in $(pwd)"
for i in $( ls -1); do
  echo "processing $i ..."
  fileext=${i##*.}
  case ${i##*.} in
    zip)
      mkdir archive
      mkdir $(pwd)/archive/${i%%.*}
      unzip $i -d $(pwd)/archive/${i%%.*}
      ;;
    *) echo "nonzip"
      mkdir ${i##*.}
      mv $i $(pwd)/${i##*.}
      ;;
  esac
done
echo "Done in $SECONDS seconds!"

I don't see where awk fits in this. I'm going to point out a few problems in your script, so you'll at least get something useful out of this. But please do tell us what you're actually trying to do.

IFS=$(echo -en "\n\b")

There's a simpler way of writing that in bash: IFS=$'\n\b'. I don't understand why you're setting $IFS to include a backspace character; and whatever need you may have found to set IFS is probably caused by other problems that I'll point out below.

echo "trigger took place in $(pwd)"

There's a variable $PWD that contains the current working directory. $PWD is equivalent to $pwd and a little faster.

for i in $( ls -1); do

Do not parse the output of ls. ls is a tool for displaying the attributes of files. The shell has a perfectly good construct for listing files in a directory: wildcards. Make this for i in *; do … This way has the advantage that it can cope with arbitrary file names; parsing the output of ls will always choke on some “strange” file names.

fileext=${i##*.}

You should use this variable below.

mkdir archive

If you run this part multiple times, the directory will already exist. Test for the directory's existence first, or call mkdir -p archive.

mkdir $(pwd)/archive/${i%%.*}

Again, $PWD. Except that there's no point in using it, a relative path would do. Also, you need double quotes around all variable and command substitutions; otherwise the shell splits the result of the substitution at $IFS characters, then interprets the pieces as glob patterns (file name wildcards) and expands them. Always put double quotes around variable and command substitutions unless you know why you need to leave them off.

Another point: ${i%%.} strips everything after the first . in the file name. This is not compatible with the definition of $fileext. To get the file except for its last extension, use ${%.} (truncate at the last .).

$(unzip $i -d $(pwd)/archive/${i%%.*})

I don't know what you're trying to do here, interpreting the output of unzip as a shell snippet. Again, double quotes around variable substitutions.

mkdir ${i##*.}

Again, double quotes. Also, if the file name sans prefix happens to begin with a -, the mkdir command will interpret its argument as an option. To avoid this, use -- to indicate the end of the option list (this is a convention that most utilities respect): mkdir -- "${i##*.}"

Also, there's a case you don't handle: what if the file name contains no .? In that case, mkdir will attempt to overwrite the file (and fail). One solution is to create the directory with a temporary name, move the file, then rename it.

Here's the fixed code:

#!/bin/bash
#File sorter
set -e    # Abort in case of error
SECONDS=0
SAVEIFS=$IFS
echo "trigger took place in $PWD"
for i in *; do
  echo "processing $i ..."
  case $i in
    *.zip)
      [ -d archive ] || mkdir archive
      mkdir "archive/${i%.*}"
      unzip -d "archive/${i%.*}" "./$i";;
    *.*)
      mkdir -p "${i##*.}"
      mv -- "$i" "${i##*.}/";;
    *)
      tmp=$(TMPDIR=. mktemp -d)
      mv -- "$i" "$tmp/"
      mv -- "$tmp" "$i";;
  esac
done
echo "Done in $SECONDS seconds!"

StackExchange Code Review Q#9662, answer score: 7