8

I'm trying to execute the code below but when I try to use my function in the if statement I get the -bash: [: too many arguments error. Why is it happening?

Thank you in advance!

notContainsElement () {
  local e match="$1"
  shift
  for e; do [[ "$e" == "$match" ]] && return 1; done
  return 0
}

list=( "pears" "apples" "bananas" "oranges" )
blacklist=( "oranges" "apples" )
docheck=1

for fruit in "${list[@]}"
do
    if [ notContainsElement "$fruit" "${blacklist[@]}" -a $docheck = 1 ]
    then
        echo $fruit
    fi
done
  • 1
    Use shellcheck.net (or its offline version). It finds the issue mentioned in the answers albeit with a far less detailed explanation and without a solution. – David Foerster Jul 14 '18 at 0:18
14

When using if [ ... ] you are actually using the [ utility (which is the same as test but requires that the last argument is ]).

[ does not understand to run your function, it expects strings. Fortunately, you don't need to use [ at all here (for the function at least):

if [ "$docheck" -eq 1 ] && notContainsElement "$fruit" "${blacklist[@]}"; then
  ...
fi

Note that I'm also checking the integer first, so that we may avoid calling the function at all if $docheck is not 1.

This works because if takes an arbitrary command and decides what to do from the exit status of that command. Here we use a [ ... ] test together with a call to your function, with && in-between, creating a compound command. The compound command's exit status would be true if both the [ ... ] test and the function returned zero as their exit statuses, signalling success.

As a style note, I would not have the function test whether the array does not contain the element but whether if does contain the element, and then

if [ "$docheck" -eq 1 ] && ! contains "$fruit" "${blacklist[@]}"; then ...

Having a function test a negative will mess up logic in cases where you do want to test whether the array contains the element (if ! notContainsElement ...).

  • Thank you very much for the answer and the other pieces of advice! – Andrea Silvestri Jul 13 '18 at 15:14
3

try

if notContainsElement "$fruit" "${blacklist[@]}" && test "$docheck" = 1
then
  • -a option is neither a shell or a test option

here you have two part test

notContainsElement "$fruit" "${blacklist[@]}"
test $docheck = 1 ## or [ $docheck = 1 ]

you link then in if using

if cmd1 && cmd2

as pointed out -a is a test option, but can only be used with other test option, thus you can use

if [ "$a" -lt "$b" -a "$a" -lt "$c" ]

to test that $a is lower than both $b and $c, but you cannot used other command within test scope.

  • arg, @Kusalananda was fastest gun in world wild web again ;) – Archemar Jul 13 '18 at 14:28
  • RSI is the price I pay for that. – Kusalananda Jul 13 '18 at 14:30
  • -a is test option, it means AND. – hyde Jul 13 '18 at 14:57
  • @hyde Except it has been marked obsolescent in the POSIX standard. – Kusalananda Jul 13 '18 at 15:04
  • 1
    @Archemar, ...consider fixing the quoting bugs still present in your examples (unquoted $docheck, unquoted $a/$b/etc in the last example). – Charles Duffy Jul 13 '18 at 17:45
0

Here is an alternative that some people may not like. Convert your blacklist array to a string, and see if the string with a fruit removed is the same. Edited to pad the strings with spaces. Thanks to Scott for pointing out the apple/pineapple problem.

badlist=" ${blacklist[@]} "
for f in "${list[@]}"
do
    if [[ "${badlist/" $f "/}" == "$badlist" ]]
    then
        echo "$f"
    fi
done

I think this is simpler, but lacks the && logic which many prefer.

  • (1) This fails in the case that one of the list fruits is contained within one of the blacklist fruits.  For example, if we change blacklist to ( "oranges" "pineapples" ), the output of your script doesn’t change. … (Cont’d) – Scott Jul 13 '18 at 18:38
  • (Cont’d) …  (2) I don’t quite understand what you mean by “I think this is simpler, but lacks the && logic which many prefer.”  It’s often easier to make things simpler by leaving out functionality.  As Albert Einstein may or may not have said, “Everything should be made as simple as possible, but no simpler.”  Why did you leave out the &&?  (3) When you post code, please indent it appropriately. – Scott Jul 13 '18 at 18:38
  • Well, I tried to use the 4-space indent but it seems that it didn't work. I see what you mean about the pineapples, though. – Wastrel Jul 13 '18 at 20:19
  • It also can't distinguish between a single list entry two words, and two subsequent items, two and words. And if your list contained * as an entry, it would match everything. (And because you aren't quoting the $f in what should be echo "$f", if you did try to echo such an element, it would be replaced with a list of filenames in the current directory). – Charles Duffy Jul 13 '18 at 22:09
  • I've made some edits. This is not trivial when the data in the arrays is arbitrary. Suppose "apples" is in the blacklist and "Granny Smith apples" is an element of the other array. I think the OP's code has similar issues. The arrays have to be made of elements that "work." – Wastrel Jul 14 '18 at 17:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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