0

I keep getting this error

line 3: Syntax error near unexpected token `|'

This is the script at the moment:

for filename in $(ls $1) | grep -v ".old"
do
    mv "$1/$filename" "$1/$filename.old"
done

Any fixes?

Rui F Ribeiro
  • 56,709
  • 26
  • 150
  • 232
  • 3
    for in ; do [ ; ]... done You probably wanted to write "$(ls "$1" | grep -v '.old')". But then you should never use the output of ls in a script; it's for human consumption only. – AlexP Dec 07 '18 at 18:20

3 Answers3

4
for filename in "$1"/*; do
    if [ -f "$filename" ] &&
       [ "${filename%.old}" = "$filename" ]
    then
        mv -- "$filename" "$filename.old"
    fi
done 

Use grep for text, and use the output of ls for reading with your eyes, never as input to a program.

The error that you get is due to a syntax error in your code. The syntax of a for loop (to loop over a number of words) is

for variable in word-list; do ...; done

The code at the top will loop over all regular files in the directory given in $1 and give them an .old filename suffix if they don't already have it. The two tests tests whether $filename refers to a regular file (or a symbolic link to one), and whether $filename, when removing .old from the end of its value, remains the same (i.e. whether it already has the suffix or not).

Another way of doing more or less the same thing is with find:

find "$1" -maxdepth 1 -type f ! -name '*.old' -exec mv {} {}.old ';'

Note that both solutions would overwrite any already existing .old file if there is a corresponding filename without the .old suffix. If there was a directory with an .old suffix under $1, the file would be put into that directory instead of being renamed.

The difference between the find solution and the shell loop is that the find solution would also care about hidden names, and that the shell loop would rename symbolic links to regular files.

Related:

Kusalananda
  • 333,661
1

You have essentially admitted that this is a homework assignment and not related to real life.  As has been written in comments, your teacher probably wants you to do

for filename in $(ls "$1" | grep -v '\.old$')

If your teacher is extremely stupid unenlightened, they may prefer that you leave out the quotes, the \ and the $.  Please introduce your teacher to Unix & Linux Stack Exchange.  But, if your teacher can't or won't learn how to do things correctly, then do the homework the way they want.

Then immediately forget it, because it's flawed.


Here's a variation on Kusalananda's answer that might be a little bit easier to read.  (I added error checking because that's the right thing to do™.)

if [ "$1" = "" ]
then
    printf 'Usage:  %s <directory-name>\n' "$0"
    exit 1
fi
if [ ! -d "$1" ]
then
    printf 'Error: %s is not a directory.\n' "$1"
    exit 2
fi
for filename in "$1"/*
do
    case "$filename" in
       (*.old)                  # Skip
        ;;
       (*)
        mv -- "$filename" "$filename.old"
    esac
done

You might also want to

  • handle the case where $1 is a directory, but either
    • you don't have full permission to it, or
    • it is empty.
  • ask whether you are supposed to rename directories, symbolic links, and other things.
1

With bash and extended file expansion:

shopt -s extglob
for file in !(*.old); do ...
glenn jackman
  • 85,964