3

I'm trying to change the files generated by an app that unfortunately often contain spaces. I have managed to get "echo" to give me output that works if copy pasted into terminal, but when I try to do the command, it does not work. I looked at this answer, which has helped me before, but even the "${x}" syntax does not seem to work in this case.

#!/bin/sh
cd ~/Data
IFS=$'\n';for i in $(ls); do
    echo "$i";
    filename="$i"
    date=$(date -n +%Y-%m-%d)
    new_filename="$date$filename"
    echo mv \'"${filename}"\' \'"${new_filename}"\'
    mv \'"${filename}"\' \'"${new_filename}"\'
done;
2
  • 1
    For the record you also had extra quotes on the mv line.
    – hildred
    Jan 28 '15 at 15:40
  • What do you mean? The echo mv and mv lines are identical, no?
    – Var87
    Jan 28 '15 at 17:56
5

The main problem is that you loop over output of ls command. Use glob * instead:

#!/bin/sh
cd ~/Data
for i in *; do
    echo "$i"
    filename="$i"
    date=$(date -n +%Y-%m-%d)
    new_filename="${date}${filename}"
    echo mv "${filename}" "${new_filename}"
    mv -- "${filename}" "${new_filename}"
done

Additionally I added -- to mv in order to treat correctly files which names begin with -. And btw my date command doesn't have -n option, but I leave it as you may have different version.

0
1

The other problem you had was that you also had extra quotes on the mv line. When you are using echo to print a command that will be copied to a shell you need quote everything twice, once for this shell, and once for the second. your echo mv worked fine as long as the filename did not begin with a dash or contain a single quote. The mv only needed quoted once.

Here is how I would rewrite your snippet.

#!/bin/bash
cd ~/Data
date="$(date -n +%Y-%m-%d)"

for filename in *; do
    echo "${filename}"
    new_filename="$date$filename"
    filename_e="$(echo "$filename"|sed -e 's!\('\''\)!\\\1!')"
    new_filename_e="$(echo "$new_filename"|sed -e 's!\('\''\)!\\\1!')"
    echo mv -- \'"${filename_e}"\' \'"${new_filename_e}"\'
    mv -- "${filename}" "${new_filename}"
done;

This version will correctly rename any file with any character in the name. the only limitation is that the echoed command may produce commands that do not work in files that have a newline in the name.

1
  • thanks, the added functionality of working with filenames containing any character is very cool!
    – Var87
    Jan 29 '15 at 1:00
-1

You just need to treat filename adding an escape caracter \ before the space.

filename=$(echo $i|sed 's/ /\\ /g')
mv $filename $new_filename
7
  • 1
    Please test your answers before posting. This will make no difference whatsoever. The problem is that $i does not contain the entire file name, each word of the name will be saved as a separate $i. You are also not quoting your variables which can cause all sorts of problems. Finally, you should also consider that not all whitespace consists of spaces. You can also have tabs, newlines etc in a filename.
    – terdon
    Jan 28 '15 at 14:42
  • They have been tested, test your comments before posting them. As you can see his code has IFS=$'\n' which makes $i get the righ name for every file SPACES INCLUDED. So the answer is perfectly right and I have tested before posting. No need to quote variables since spaces have been escaped before. Be sure when you make a comment correcting something you are right before you place the comment.
    – YoMismo
    Jan 28 '15 at 14:50
  • 1
    Well, the question had IFS=$'\n', your answer did not, so I didn't realize. Still, you're right, my comment was more aggressive than was necessary, sorry. Nevertheless, this will still break if the whitespace is not a simple space and your sed will only escape the first space.
    – terdon
    Jan 28 '15 at 15:16
  • You're right about multi spaces, I have added the ending 'g' to substitute all of them in case there are more than one.
    – YoMismo
    Jan 28 '15 at 15:21
  • Thanks, but this still breaks on tabs, newlines, carriage returns, file names beginning with - etc. It is also needlessly complicated and creates problems where there were none by parsing ls. The Right Way© is to either use shell globbing or find -print0 instead.
    – terdon
    Jan 28 '15 at 15:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.