Changing filename and copying in bash

Question

I wrote a script which changes names of last downloaded songs and copy them to music directory. Can it be written better?

    #!/bin/bash
IFS=$'\n'
for file in $(find "/home/stanek/Downloads/" -iname "*.mp3" -mtime -7 | sort)
do
    nowy=$(echo $file \
    | sed "s/_/ /g" \
    | sed "s/\s*(*[A-z0-9.]*.pl)*//g" \
#leave brackets and text if it is remix
    | sed "s/\(\s*(\s\?[A-z0-9 ]*remix)\)\|\s*(\s\?[a-z0-9. ]*)/\1/gi")

    if [ "$nowy" != "$file" ]
    then
        echo "ZMIANA NAZWY DLA:"
        echo $nowy
        mv $file $nowy
    fi
done

for file in $(find "/home/stanek/Downloads/" -iname "*.mp3" -mtime -7 | sort)
do
    echo "I will copy: $file" | sed "s/\/home\/stanek\/Downloads\///g"
done
read -p "Is it ok? (y)" -n 1 -r
echo
if [[ ! $REPLY =~ ^[Yy]$ ]]
then
    exit 1
fi

i=0

for file in $(find "/home/stanek/Downloads/" -iname "*.mp3" -mtime -7 | sort)
do
    paste=$(echo $file | sed "s/\/home\/stanek\/Downloads\///g")
    cp $file "/home/stanek/Music/$paste"
    i=$((i+1))
done
echo "Copied $i files"

Answer 1

"/home/stanek/Downloads/" is used often, consider putting that as a script variable, if it's not already present as an environment variable provided by your shell user profile? Let's assume USER_DOWNLOADS="/home/stanek/Downloads", MUSIC_DIR="/home/stanek/Music" for the suggestions below.

You are invoking find three times, the problem being that new files created during each invocation will be missed by the subsequent one. This may be less of a problem between the first two, but will be for the second and third loops that are separated by a user prompt.

The other question is ask is whether your renaming of files should be done as part of the copying (i.e. later) process, or do you always want to rename the files regardless of whether the copying is done.

If it's the former, then you should only construct the new name within the final loop, for example:

# not *exactly* sanitized for files with spaces, just used as illustration only!
files_to_copy=$(find $USER_DOWNLOADS -iname "*.mp3" -mtime -7 | sort)
echo "Files will be copied:"
for i in $files_to_copy; do basename $i; done
# prompt user
for i in $files_to_copy; do
    new_name=$(basename $i | sed -e 's/.../.../;...');
    cp $i $MUSIC_DIR/$new_name
done

You can string sed commands together: sed -e 's/.../.../;...' as shown above. basename will do the equivalent of your sed "s/\/home\/stanek\/Downloads\///g".

If it's the latter, i.e. same as what you have presented here, then you can leave the first loop aside, but build up your list of files to be copied first:

files_to_copy= # ...
# ...
# prompt user
cp $files_to_copy $MUSIC_DIR
echo "$(awk -F' ' '{print NF}' <<< $files_to_copy) file(s) copied."

To get past filenames with spaces in them, files_to_copy should be constructed as a variable array.

Answer 2

In addition to the other excellent suggestions,

Instead of using sed:

$(echo $file | sed "s/\/home\/stanek\/Downloads\///g")

try bash parameter substitution:

"${file//\/home\/stanek\/Downloads\//}"

And remember to quote - there's a few places that have a bare $variable that shouldn't be unquoted.