0

I am making a script that should play any media files in the downloads folder and shred it if I do not want to keep them. It works on the file types .swf, .webm, .gif, .png but not .jpg. For jpg I get this error

 '[: too many arguments' 

If I change it to .png without changing anything else then it works.

I have tried to change it from *jpg to *png, and that works. Changing it back to *jpg, gods forbid that. I can't find anything on google that can help me with this.

#!/bin/bash

cd Downloads

get_files () {
    for i in *.*; do
    [ -f "$i" ] || break

    echo "Playing '$i'"

    if [ "$i" == *swf ]; then
        ./flashplayer $i
        shred_file $i
    elif [ "$i" == *webm ]; then
        vlc $i
        shred_file $i
    elif [ "$i" == *gif ]; then
        xdg-open $i
        shred_file $i
    elif [ "$i" == *jpg ]; then
        xdg-open $i
        shred_file $i
    elif [ "$i" == *png ]; then
        xdg-open $i
        shred_file $i
    fi
done
}

shred_file () {
    echo ""
    echo "Do you want to shred the file?"

    read r

    if [ "$r" == "y" ]; then
        shred -uvz $1
    else
        echo keep
    fi
}

get_files

I expect this script to be able to open .jpg files and any other file types defined in this script. I do not expect this error to occur at all

papanito
  • 2,349
  • 2
  • 32
  • 60
  • 1
    Your succession of `elif` is best replaced with `case "$i" in....` – Léa Gris Sep 07 '19 at 16:38
  • The error message says "'[: too many arguments'". So, the obvious first question would be: how many arguments *are* you passing? – Jörg W Mittag Sep 07 '19 at 16:43
  • Possible duplicate of [Meaning of "\[: too many arguments" error from if \[\] (square brackets)](https://stackoverflow.com/q/13781216/608639), [When to wrap quotes around a shell variable?](https://stackoverflow.com/q/10067266/608639), etc. Also see [How to use Shellcheck](http://github.com/koalaman/shellcheck). – jww Sep 07 '19 at 16:44
  • I didn't really think about what *jpg was really doing. Now I know what it really means. – RexTheCapt Sep 07 '19 at 16:45
  • I second the recommendation of `case`. For one thing, it directly supports wildcard matching (like `*.jpg`). For another, it's specifically designed for doing a series of "if it matches this, then do that" conditions. – Gordon Davisson Sep 07 '19 at 17:26

2 Answers2

1

bash will expand *jpg to all the file names which end 'jpg'. And so on for all the file suffixes. The '.jpg' test causes an error probably because there are a number of files that match *.jpg.

Better to test the suffix itself:

suffix=${i#*.}  # if $i is file.jpg yields 'jpg'

And then do tests for the suffix value:

 if [ "$suffix" == "swf" ]; then
      ./flashplayer $i
      shred_file $i
 elif [ "$suffix" == "webm" ]; then
      vlc $i
      shred_file $i
    ...
    ...

Incidentally, for the for loop, you could (if appropriate) specify the file types the loop should process:

    shopt -s nullglob
    for i in *.{swf,gif,png,jpg,jpeg}; do
suspectus
  • 16,548
  • 8
  • 49
  • 57
  • 1
    if you don't `shopt -s nullglob` and there is no file with the extensions, `for i in *.{swf,gif,png,jpg,jpeg}; do` will iterate the expanded list of literal patterns, as the values for `$i`: `'*.swf' '*.gif' '*.png' '*.jpg' '*.jpeg'`. – Léa Gris Sep 07 '19 at 16:53
  • 1
    To test without `nullglob`: `shopt -u nullglob; mkdir -p t; touch t/foo.gif t/bar.swf t/baz.swf; for i in t/*.{swf,gif,png,jpg,jpeg}; do echo "$i"; done; shopt -s nullglob`. Output: `t/bar.swf t/baz.swf t/foo.gif t/*.png t/*.jpg t/*.jpeg ` – Léa Gris Sep 07 '19 at 17:30
1

In Bash (and other POSIX shells), [ ... ] is not particularly magical; in fact, it can be implemented as a completely separate program, though in practice most shells do provide it as a builtin. (If you run type -a [, you'll most likely see that your system has both a builtin and a separate program.)

So the problem is that if your current directory contains files whose names end with jpg, such as foojpg and barjpg, then this command:

[ "$i" == *jpg ]

expands to something like this:

[ my.file == foojpg barjpg ]

which is obviously not what you want.

And even if you escaped the * to prevent the filename-expansion ([ "$i" == \*jpg ] or [ "$i" == '*jpg' ]), it still wouldn't do what you want, because [ ... ] doesn't support this sort of glob comparison.

Since you're specifically using Bash, your best bet is to use its special [[ ... ]] syntax, which is magical, and has special support for globs:

[[ "$i" == *jpg ]]

(And likewise for all of your other tests.)

ruakh
  • 175,680
  • 26
  • 273
  • 307
  • So it finds every single files that ends with *jpg and makes an array out of that? – RexTheCapt Sep 07 '19 at 16:39
  • @RexTheCapt: Not an array -- it's just the normal filename expansion (https://www.gnu.org/software/bash/manual/bash.html#Filename-Expansion), the same thing you're relying on when you write `for i in *.*` and expect Bash to replace `*.*` with all the filenames that contain dots. – ruakh Sep 07 '19 at 16:41
  • When you iterate a globbing pattern, `shopt -s nullglob` to avoid dealing with the pattern itself as an entry when no filename match. – Léa Gris Sep 07 '19 at 16:44