Home / ubuntu / Questions / 1108810
Accepted
singrium
Asked: 2019-01-12 02:09:22 +0800 CST

shell script to remove files didn't work

  • 772

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.

scripts command-line bash

3 Answers

  1. Best Answer

    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.

    • 11

  2. 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.

    • 6

  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.

    • 3