One possible way to do it
A while back I created a function specifically for that purpose which I keep in my .bashrc
, and can be adapted into a script. You should be taking advantage of positional parameters so that users can put filenames on command-line. Here's my original function:
swap_files() {
if [ $# -ne 2 ]
then
echo "Usage: swap_files file1 file2"
else
local TMPFILE=$(mktemp)
mv -- "$1" "$TMPFILE"
mv -- "$2" "$1"
mv -- "$TMPFILE" "$2"
fi
}
You can get rid of swap_files(){
declaration, local
keyword, and closing }
, and turn it into a script - just add #!/bin/bash
at the top. Granted there's tons of things that can be improved, but at the very basic level that's about as simple as swapping gets (which by the way is frequently taught in C to swap array items, but that's just a tangent topic).
#!/bin/bash
if [ $# -ne 2 ]
then
printf "Usage: swap_files file1 file2\n" > /dev/stderr
else
TMPFILE=$(mktemp)
mv -- "$1" "$TMPFILE"
mv -- "$2" "$1"
mv -- "$TMPFILE" "$2"
fi
Of course, remember to quote the positional parameters if filenames contain spaces. Like so:
swap_files 'file 1' 'file 2'
Note the use of --
to avoid issues with filenames that have leading -
in them. A better way would be to get into habit of referencing files in current working directory with ./
, especially if you are using globstar *
( globstar is not relevant in this question, but it's worth mentioning if we're talking about filenames with leading -
). Besides,./
way is more portable, since some versions of mv
such as on FreeBSD don't have the --
option.
As suggested by terdon in the comments, we can also create temp file in first file's parent folder to avoid moving files across filesystems.
#!/bin/bash
if [ $# -ne 2 ]
then
printf "Usage: swap_files file1 file2\n" > /dev/stderr
else
file1_dir=${1%/*}
# just in case there were no slashes removed, assume cwd
if [ "$file1_dir" = "$1" ]; then
file1_dir="."
fi
tmpfile=$(mktemp -p "$file1_dir" )
mv -- "$1" "$tmpfile"
mv -- "$2" "$1"
mv -- "$tmpfile" "$2"
fi
Your script and things to improve
1. Redundant variable assignment
file1=$file1
file2=$file2
This portion assigns $file1
variable to ... file1
variable; there's two problems with this - assigning variable to itself is redundant, and it doesn't exist to begin with, there's no declaration of that variable earlier in the script.
Here's what's going to happen if your user tries to put in even quoted items into your read
command:
$ read file1 file2
'one potato' 'two potato'
$ echo "$file1"
'one
$ echo "$file2"
potato' 'two potato'
In accordance with shell behavior, the shell splits everything that is read on stdin
and tries to fit in each word into corresponding variables, and if words exceed number of variables - it tries to push everything into the last variable. I'd recommend you read in each file , one at a time.
3. Copying to itself is an error
You're doing
cp $file1 $file1;
That'll produce an error
$ cp input.txt input.txt
cp: 'input.txt' and 'input.txt' are the same file
Perhaps you wanted to do
cp "$file1" "$file1".tmp
Or just make use of mktemp
command as I did. Also note the quoting of variables to prevent word splitting.
Other fun ways to do it
Did you know that you can cat any file with redirection to make a copy? So using mv
or cp
isn't the only way. Something like this:
$ cat ./wallpaper.jpg > wallpaper.jpg.tmp
$ cat ./screenshot.jpg > wallpaper.jpg
$ cat ./wallpaper.jpg.tmp > ./screenshot.jpg
file1=$file1
? Did you meanfile1=$1
?cp $file1 $file1
-- you're copying a file to itself.