# Changing filename and copying in bash

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"


"/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.

• @siema glad to help! – h.j.k. Jun 10 '15 at 0:36

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.

• Bash substitution is a good all-pupose suggestion, but in the context of this question, basename will be more appropriate as the intention is to get the filename. :) Have +1 anyways. – h.j.k. Jun 12 '15 at 10:29
• @h.j.k. - quite right; I hadn't spotted how the substitution was being used! – Toby Speight Jun 15 '15 at 13:44