There's a bunch of small items to consider, and then some larger items too. Starting small....
Sweating the small stuff....
cd ~/
can be shortened to just cd
. Not a big deal one way, or the other, but still....
Try to quote string values, even when they do not have spaces. Your basepath
variable may end up having a space in it at some point ("My Downloads" at some point?), and the quotes make it clear. Again, this is a minor item...
Having declared the basepath
variable, why are you not using it? Here you have:
filename=$(inotifywait -e create --format "%f" Downloads/InternetDownloads)
which should be:
filename=$(inotifywait -e create --format "%f" "$basepath")
... note the quotes around the variable in case it ends up with spaces again.
File Extensions
You pull the extension off the file with: arr=(${filename//./ })
. This replaces the dots in the filename, with spaces, then loads that space-separated list in to an array (which considers the spaces to be item-separators). Then, you pull the extension from the array with ${arr[1]}
....
Ouch. This works only if there's only one .
in the file name, and the file has no spaces in the name.
There's a much simpler pattern-match you can do to alleviate this problem:
extension=${filename##*.}
From the documentation:
the result of the expansion is the expanded value of parameter with the shortest matching pattern (the #
case) or the longest matching pattern (the ##
case) deleted.
With that, there's no arrays, and the extension is the last part (or the whole filename if there are no '.' values in the file.... you need to check for that case too. There's also the possibility that a file may end with a '.', leaving an empty-value extension.... you need to check that too.
inotify
Currently you have a number of problems with inotify:
you are letting it dump messages to STDERR:
Setting up watches.
Watches established.
This is repeated for each file. It's unnecessary, and can be removed with the -q
option to inotifywait.
You are using the create
event to track. This is triggered when the file is created... but is that what you want? A download may take a while to complete, so moving the file part way through a download may not make sense. Many programs will continue based only on the filehandle, which does not depend on the file location, but not all programs will behave well if the file is moved part way through.
You should consider tracking the close_write
event instead. This will only trigger when a file handle is clsed after a write to the file handle. I expect this will track the 'end of download' rather than the 'start of download'
inotifywait
(the way you have it set up) will only notify when the first file is created. This leaves you open to race conditions where a download may happen when you are copying the file to a new location. This new download will not be triggered on the next inotifywait
cycle because it started before the wait. As a result, you are at risk of losing events.
I believe the best strategy for your job would be to declare a function for the jobs you want to do with each file.... and then pipe a 'monitor' stream in to it. I have put together the following:
#!/bin/bash
function relocate {
local filename=$1
local extension=${filename##*.}
# need -d to text for directories we ourselves create
if [[ -d "$filename" || "$extension" = "" || "$extension" = "$filename" ]]
then
# Directory, No extension, or empty extension.
echo "Skipping: $filename"
return
fi
if [ ! -d "$extension" ]
then
echo "Creating directory: $extension"
mkdir "$outdir/$extension"
fi
echo "Relocate: $filename -> $outdir/$extension"
mv "$filename" "$outdir/$extension/"
}
path=${1:-.}
outdir=${2:-.}
cd $path
echo "Working in $PWD with relocate to $outdir"
inotifywait --quiet --monitor --event close_write --format "%f" . | while read line
do
relocate "$line"
done