0

Trying to comparisons with a variable against strings, i tried code as per the solution from https://unix.stackexchange.com/questions/67898/using-the-not-equal-operator-for-string-comparison

if [ "$ACTION" != "dry" ] && [ "$ACTION" != "prune" ]
then
    echo "Invalid"
fi

This does not work for me, i get no error messages it's like it just skips the code block.

I have also tried like this as per answer here Bash if statement with multiple conditions

if [[ "$ACTION" != "dry" && "$ACTION" != "prune" ]]
then
    echo "Invalid"
fi

This echoes "Invalid" if $ACTION is anything other than "dry", even if its "prune"

Any suggestions?

EDIT

Full code

OPTIND=1
while getopts "b:a:" option
do
    case "${option}"
    in
        b) MERGE_BRANCH=${OPTARG};;
        a) ACTION=${OPTARG};;
    esac
done

if [[ "$ACTION" != "dry" && "$ACTION" != "prune" ]]
then
    echo "Invalid"
fi

shift $((OPTIND-1))
[ "$1" = "--" ] && shift

(( 1 <= ${#} )) || { echo "missing mandatory argument" 2>&1 ; exit 1;     };
codeforester
  • 39,467
  • 16
  • 112
  • 140
A.Jac
  • 1,443
  • 3
  • 17
  • 24
  • 1
    How does `ACTION` get set? What is the output of `declare -p ACTION`? I suspect the value has trailing whitespace (specifically, a carriage return). – chepner Feb 03 '17 at 14:19
  • @chepner declaring action outputs correctly `declare -- ACTION="prune"` It is set with getopts like this `while getopts "b:a:" option do case "${option}" in b) MERGE_BRANCH=${OPTARG};; a) ACTION=${OPTARG};; esac done` – A.Jac Feb 03 '17 at 14:25
  • This echoes "Invalid" if $ACTION is anything other than "dry", even if its "prune" → not for me: it does not print "Invalid" if `ACTION` equals `prune`. – rom1v Feb 03 '17 at 14:25
  • @A.Jac Post the *exact* code you run to get the output you claim, instead of assuming that the valid code you do post will produce the same problem. – chepner Feb 03 '17 at 14:27
  • Edited OP with full code – A.Jac Feb 03 '17 at 14:30
  • `tmp.sh -a prune` does not print `Invalid` with the code you posted above. – chepner Feb 03 '17 at 14:41

2 Answers2

3

Well, in most situations where such a command arises, using case provides a more maintainable solution, because during the life cycle of your script you will want to change action names, deal with new situations, and so on:

case "$ACTION" in 
  ("dry"|"prune") 
    : # Insert appropriate code
  ;;
  (*)
    echo Invalid
  ;;
esac

Second, there is no need to use the [[ syntax in this case

if [ "$ACTION" != dry ] && [ "$ACTION" != prune ]; then
  echo Invalid
fi

is enough in this case. To test that everything is working as expected, you can add an else branch:

if [ "$ACTION" != dry ] && [ "$ACTION" != prune ]; then
  echo Invalid
else
  echo Valid
fi
Dario
  • 2,673
  • 20
  • 24
  • `-a` is obsolete and should not be used in new code. Use `[ ... ] && [ ... ]` instead. – chepner Feb 03 '17 at 14:42
  • `!=` does not perform a regular expression match; it performs a pattern match *if* the RHS contains an unquoted pattern. – chepner Feb 03 '17 at 14:43
  • I got it to work using `case` and moving the check after the `shift` operation. This seems to work as intended, updating OP with solution – A.Jac Feb 03 '17 at 14:47
  • @chepner Thank you for your comments. I corrected my answer accordingly. – Dario Feb 03 '17 at 14:57
  • 1
    @A.Jac That's actually not a good practice. The question should be just that, a question - and not a question _and_ an answer. If what you've added is just this answer, you can just remove it; if you've found a different solution, you should add it as a proper answer. – Benjamin W. Feb 03 '17 at 15:01
  • @BenjaminW. Okay, since i cant delete i will add my solution as an answer – A.Jac Feb 03 '17 at 15:04
  • @A.Jac You can't edit the question? Never mind, looks like you just did. – Benjamin W. Feb 03 '17 at 15:04
  • @BenjaminW. I thought you meant remove the entire question. – A.Jac Feb 03 '17 at 15:05
  • @A.Jac Oh, no, I just meant not adding an answer to the question body. – Benjamin W. Feb 03 '17 at 15:05
0

Solution

Using case as per @Dario's answer and moving the check to after the shift operation makes the code work as intended.

OPTIND=1

while getopts "b:a:" option
do
    case "${option}"
    in
        b) MERGE_BRANCH=${OPTARG};;
        a) ACTION=${OPTARG};;
    esac
done

shift $((OPTIND-1))
[ "$1" = "--" ] && shift

(( 1 <= ${#} )) || { echo "missing mandatory argument" 2>&1 ; exit 1; };

case "$ACTION" in
  (dry|prune)
    :
  ;;
  (*)
    echo "Invalid argument"
    exit 1
  ;;
esac
A.Jac
  • 1,443
  • 3
  • 17
  • 24