syntax error in expression in a bash script

Question

I have the following bash script:

files=[ls .]
for file in $files
 do 
 digits=$(echo $file | sed "s/[^0-9]*\([0-9]\+\).*/1円/p")
 if [[ $digits -gt 1000 ]]
 then
 rm $file 
 fi 
done

Why am I getting:

11923: syntax error in expression (error token is "11923")

where 11923 is an example of one printed line when the $digits variable has value 11923?

EDIT:

I have found the error. The debuger says that the value of the $digits is duplicated (ex '1001 1001'). I am not sure why this is happening, though. The output of the debugger:

+ for file in *
++ echo image10961.jpg
++ sed 's/[^0-9]*\([0-9]\+\).*/1円/p'
+ digits='10961
10961'
+ [[ 10961
10961 -gt 1000 ]]
test.sh: line 5: [[: 10961
10961: syntax error in expression (error token is "10961")

• Cyrus: This might help: How to debug a bash script?
• Cyrus: Make sure that there is always a number in variable $digits.
• sjsam: What is contained in $files? Is it a list of files? In any case you should be using an array, or more easily globing
• Aleksandar Jovanovic: $files is indeed a list of files, and $digits are always numbers. I have checked that sed does return a number, as all the files have the same pattern.
• sjsam: @AleksandarJovanovic : Could you show how you're populating the $files variable? Add the code please..

Answer 1

You can't use the ls to populate the files as noted in [ this ]

The easiest way is to use globbing ie

for file in ./*
do
 # do something with $file
done

You may use the extended regex with sed

digits=$(echo "$file" | sed -E "s/[^0-9]*([0-9]+).*/1円/p")

and note that $file is double quoted.

Regarding the error

11923: syntax error in expression (error token is "11923")

It is because you have added a p flag after the sed substitution. Change it to

digits=$(echo "$file" | sed -E "s/[^0-9]*([0-9]+).*/1円/")

By default sed prints stuff by default unless you use -n option

Answer 2

The immediate problem is here:

files=[ls .]

This should use $(...) to execute a command and capture its output.

files=$(ls .)

Though, really, it's best to avoid parsing the output of ls. Better to get rid of $files and use * to loop over all the files in the current directory.

for file in *

• Aleksandar Jovanovic: The error still persists. If I exclude the if clause, and just echo $digits somwehere, everything works fine. Meaning that digits are extracted well, though I am definitely not using the best practices (sorry for that).

Answer 3

Your code can be simplified like this:

for file in *; do
 [[ $file =~ ([0-9]+) ]] && (( 10#${BASH_REMATCH[0]} > 1000 )) && rm "$file" 
done

It doesn't invoke sed for each file so it's more efficient. You can use a more advanced regex if this isn't enough.

Answer 4

Here, this script should work in your case

#!/bin/bash
for file in ./f*
do
 digits=$(echo -n $file | sed -n -e "s/[^0-9]*\([0-9]\+\).*/1円/p")
 if [ $digits -gt 1000 ]
 then
 rm $file
 fi
done

Parsing the returned result in ls is tricky as mentioned in another answer. Here is a few things I changed to make it work.

First, echo -n, which will not to print extra new line character. Then sed -n, quiet mode for sed which will not print non matching lines Last, more importantly, [ has different syntax than [[ . If you want to use -gt, you need to use [.

• John Kugelman: There's no need to suppress the newline with echo -n. Same with sed -n: this is a single file name, so using p and -n is unnecessary. "If you want to use -gt, you need to use [." Not true. [[ supports -gt.