Hacker News Reader

Comment by LukeShu | Hacker News Reader

Dissecting a bad oneliner:

    ls *.jpg|while read i;do gm convert $i -resize "900x" -unsharp 2x0.5+0.5+0 -quality 98 `basename $i .jpg`_s.jpg;done

The part I take issue with is the unnecessary invocation of `ls`. The shell does the glob expansion. A `for` loop is better suited for this. Also, if using bash,* we can get rid of basename.

    for file in *.jpg; do gm convert $file -resize "900x" -unsharp 2x0.5+0.5+0 -quality 98 ${file%.jpg}_s.jpg; done

* zsh fans, I'm sure it works there too.

View on HN

sysadmin...sense...tingling...

Get in the habit of putting $file (and, in this case, ${file%.jpg}_s.jpg) in double quotes. Your one-liner (and theirs) will blow up spectacularly if there's whitespace in a filename:

    bash-3.2$ for i in 'lots       of spaces'.jpg '1 2'.jpg; do printf 'arg = %s\n' ${i%.jpg}_s.jpg; done
    arg = lots
    arg = of
    arg = spaces_s.jpg
    arg = 1
    arg = 2_s.jpg

    bash-3.2$ for i in 'lots       of spaces'.jpg '1 2'.jpg; do printf 'arg = %s\n' "${i%.jpg}_s.jpg"; done
    arg = lots       of spaces_s.jpg
    arg = 1 2_s.jpg

EDIT: It's also worth noting that, by default, zsh doesn't split on whitespace in parameter expansion like sh, bash, etc. do. ("man zshexpn" and search for "SH_WORD_SPLIT")

As long as we're bashing, it could easily be written loopless like so:

    basename -s .jpg -a *jpg | 
      xargs -I{} convert "{}.jpg" \
       -resize "900x" -unsharp 2x0.5+0.5+0 -quality 98 "{}_s.jpg"

EDIT: weird line breaking is to prevent sidescroll box

You could also easily parallelize this with xargs' -P option.

Reply on news.ycombinator.com