3

I want to delete multiple images having resolution less then 228x228. For that, I wrote this shell script:

#!/bin/bash

for i in $( ls ); do
    if [$(identify -format "%w" $i) < 228] && [$(identify -format "%h" $i) < 228];
    then
        rm $i
    fi
done

For some reasons, I got this output when I run it:

./del.sh: line 4: [640: command not found
./del.sh: line 4: [550: command not found
./del.sh: line 4: [315: command not found
...

Could you please tell me what's wrong in this script and how to fix it.
Thank you.

EDIT: Even after I added spaces after the brackets, I still got an error. It was due to the usage of < instead of -lt and has been fixed. Now there is no error.

  • 4
  • 1
    You need spaces after [ and before ]. – Ralf Jan 11 at 14:04
  • 1
    Possible duplicate of what is wrong with this piece of code? – Barmar Jan 11 at 16:35
  • @PerlDuck Thanks? I only showed up because this was in Hot Questions, don't expect me to show up regularly. – Barmar Jan 11 at 22:35
  • 1
    Please never use the output of $( ls ) in a script, because ls doesn't have a universal standard. It's anyway more readable and more efficient in the loop to use for i in *; do. – Paddy Landau Jan 15 at 10:58
11

Some issues here: Firstly, the expression in a […] test needs spaces around it (pitfall #10), and secondly the comparison < doesn't work with […] tests (pitfall #7). You either need -lt (less than) or use [[…]] instead, which is a bashism. Also, the for loop should be replaced (pitfall #1).

So:

for i in ./*; do
    if [ -e "$i" ]; then
        if [ $(identify -format "%w" "$i") -lt 228 ] && [ $(identify -format "%h" "$i") -lt 228 ];
        then
            rm -- "$i"
        fi
    fi
done

You may also want to avoid calling identify twice to get the two dimensions (pitfall #58) but call it just once instead and let it print a string ready to be used as variable assignments in shell syntax.

If we write

identify -format "width=%w height=%h" "$i"

it will print something like width=50 heigth=250. When we evaluate that string then we have set two variables with just one call and the condition can be written as:

eval "$(identify -format "width=%w height=%h" "$i")"
if [ $width -lt 228 ] && [ $height -lt 228 ];
then
    rm -- "$i"
fi

See also: common bash pitfalls.

6

Instead of a loop, I'd use find with -exec and -delete:

find . -maxdepth 1 -type f  \
   -exec sh -c '
       [ $(identify -format "%w" "$1") -lt 228 ] &&
       [ $(identify -format "%h" "$1") -lt 228 ]' _ {} \; \
   -delete -print

This will also print the files that get deleted, you can remove -print if you don't want that.

  • 1
    "This will also print the files that get deleted, you can remove -print if you don't want that." Or remove the -delete and run it once as a dry run to sanity check before restoring it and running for real. – Kevin Jan 11 at 23:09
3

Not intended to answer but to give a useful tip that helped me much doing bash scripting.

There's a shell script linter called shellcheck that might trap some common errors in bash scripts and also avoid some pitfalls. It can be installed like any package in ubuntu -> https://launchpad.net/ubuntu/+source/shellcheck is in universe for current stable.

This is the output for your script

shellcheck del.sh

In del.sh line 4:
    if [$(identify -format "%w" $i) < 228] && [$(identify -format "%h" $i) < 228];
    ^-- SC1009: The mentioned parser error was in this if expression.
       ^-- SC1073: Couldn't parse this test expression.
        ^-- SC1035: You need a space after the [ and before the ].
                                          ^-- SC1020: You need a space before the ].
                                          ^-- SC1072: Missing space before ]. Fix any mentioned problems and try again.

If you fix and apply again you'll get some other recommendations and fixes already mentioned in the accepted answer.

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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