Why doesn't this fairly simple "if" work?

Question

Here is my script:

if [[ "$(echo "$2" | sed 's/.two//g')" == "load" ]] && [[ "$1" == "Decrypt" ]] || [[ "$(echo "$2" | sed 's/.two//g')" == "load" ]] && [[ "$1" == "Encrypt" ]] 
then
    key=aNXlye1tGbd0uP
else
    if [ -z "$key" ]
    then
        key="$2"
    fi
fi

It's supposed to look for the second argument, remove potential .two, and then compare it to load, if it is load then it should set key to aNXlye1tGbd0uP. However, this doesn't work. This is what it looks like when I run it.

pskey Decrypt load (some string)

Here is the output from bash -x:

++ echo load
++ sed s/.two//g
+ [[ load == \l\o\a\d ]]
+ [[ Decrypt == \D\e\c\r\y\p\t ]]
+ [[ Decrypt == \E\n\c\r\y\p\t ]]
+ '[' -z '' ']'
+ key=load

However, If I remove whats after [[ "$1" == "Decrypt" ]], it works. What is wrong with that line?

Answer 1

If I understand you correctly, you are looking for something like this:

if [[ "$(echo "$2" | sed 's/.two//g')" == "load" && "$1" == "Decrypt" ]] || 
   [[ "$(echo "$2" | sed 's/.two//g')" == "load" && "$1" == "Encrypt" ]]
then
    ...
fi

Note that you could also simplify the whole thing to:

 if [[ "$(echo "$2" | sed 's/.two//g')" == "load" && "$1" =~ (De|En)crypt ]]; then ...

Answer 2

In your code:

if    [[ "$(echo "$2" | sed 's/.two//g')" == "load" ]] &&
      [[ "$1" == "Decrypt" ]]                          ||
      [[ "$(echo "$2" | sed 's/.two//g')" == "load" ]] &&
      [[ "$1" == "Encrypt" ]] 

The sed call could be simplified to just: ${2%?two} if the replacement is at the end of the variable $2. Please understand that the "any character" that the dot (.) represents in sed, is equivalent to the question mark (?) in patterns (thanks @terdon). If the replacement needs to be done for all occurrences of .two, then we should use: "${2//?two}". Then we get this shorter version:

if [[ "${2//?two}" == "load" ]]  &&  [[ "$1" == "Decrypt" ]]  ||
   [[ "${2//?two}" == "load" ]]  &&  [[ "$1" == "Encrypt" ]] 

which is doing if A && B || C && D.

When A is true (load = load) B is executed.
If B is true (Decrypt = Decrypt) the following || phrase (C) is skipped and
then D is executed (Decrypt = Encrypt).
Which results (the last command executed) in a false value.
Then the else is executed .....

I suspect that what you mean is if ( A && B ) || ( C && D ) which, as A is the same as C, is exactly the same as if ( A && B ) || ( A && D ), which can be simplified (using the distributive property) to
if A && ( B || D ):

if      [[ "${2//?two}" == "load" ]] &&
      ( [[ "$1" == "Decrypt" ]]   ||  [[ "$1" == "Encrypt" ]]  );
then
      key=aNXlye1tGbd0uP
else
      if    [ -z "$key" ]
      then  key="$2"
      fi
fi

The -z test for "$key" could be simplified to a simple expansion: key="${key:-$2}"

And, maybe, it would be more readable (IMO) like this:

if A; then
      if   B || D; then

Which translates to this:

if           [[ "${2//?two}" == "load" ]]
then   if    [[ "$1" == "Decrypt" ]]   ||  [[ "$1" == "Encrypt" ]]
       then  key=aNXlye1tGbd0u
       else  key="${key:-$2}"
       fi
else         key="${key:-$2}"
fi

Or could be, using @terdon’s idea, written as this:

if           [[ "${2//?two}" == "load" ]] &&
             [[ "$1" =~ (De|En)crypt ]]
then         key=aNXlye1tGbd0u
else         key="${key:-$2}"
fi

Please Note that this is also equivalent:

if       [[  ( "${2//?two}" == "load" ) &&
             ( "$1" =~ (De|En)crypt )
         ]]
then         key=aNXlye1tGbd0u
else         key="${key:-$2}"
fi

The parenthesis are not strictly needed, but added to enforce the idea that inside [[ test you could give structure to your tests adding whitespace (tab, space, newline) and parenthesis. That does not work the same in [ tests.

Answer 3

terdon’s statement «That is very far from a “simple” if.» is an understatement.  As John Kugelman pointed out, && and || have equal precedence, and are handled from left to right.  BinaryZebra offered the idea of translating Boolean expressions into cascading if statements.  My best attempt to “simplify”

if A && B || C && D
then
        result=0
else
        result=1
fi

is

if A
then
        if B
        then
                if D                                        ←
                then
                        result=0
                else
                        result=1
                fi
        else
                if C
                then
                        if D
                        then
                                result=0
                        else
                                result=1
                        fi
                else
                        result=1
                fi
        fi
else
        if C
        then
                if D
                then
                        result=0
                else
                        result=1
                fi
        else
                result=1
        fi
fi

Very far from simple.  As you may be able to see from the above, the reason why it doesn’t work is that, (on the marked line) if A and B are both true, it still goes on to test D ([[ "$1" == "Encrypt" ]]).

As BinaryZebra shows without mentioning, you can use some math-like notation in sh/bash scripts; e.g., you can say

if (A && B) || (C && D)
then
  ︙ 

or, in the slightly simplified version of your code,

if ( [["$2" == "load" ]] && [[ "$1" == "Decrypt" ]] )  ||  ( [[ "$2" == "load" ]] && [[ "$1" == "Encrypt" ]] )
then
  ︙ 

A feature of this is that code in parentheses is run in a subshell.  If that’s a problem, you can use braces instead:

if { [["$2" == "load" ]] && [[ "$1" == "Decrypt" ]] ; }  ||  { [[ "$2" == "load" ]] && [[ "$1" == "Encrypt" ]] ; }
then
  ︙ 

Note that there must be a ; before the } (optionally separated by whitespace), and there must be whitespace before and after the {, and after the }.

Of course you should use the techniques presented in the other answers for simplifying your test, such as not doing the same test on $2 twice.

Related: Pipes & redirection binding precedence with disjuncts, conjuncts etc.? and When is “if” not necessary?

Answer 4

In the first place:

[[ $1:$2 =~ ^(En|De)crypt:(.two)*load(.two)*$ ]] && echo conditions met

But your if boolean logic is backwards:

#sed    #En=$1   #De=$1
true  && false || true    # passes your logic
true  && true  || NA      # passes your logic
false && NA    || false   # fails  your logic
false && NA    || true    # fails  your logic

The second && is omitted here because its not a factor - its result is always the same as the first. Its a noop in this logic chain.

That doesn't work well - the one failure case breaks your statement. In any of these cases in which the first test is false, you wind up ORing its return as opposed to that of the (De|En)crypt tests. And, in fact, the first of these doesn't even occur because it is short-circuited by the load return. So you want to do the ORs first, and take advantage of the short-circuit by delaying the expensive command substitution until you definitely need to test for it:

#En=$1  #De=$1   #sed
false || false && NA
false || true  && false
false || true  && true
true  || NA    && false
true  || NA    && true

All of these pass your logic and the command substitution isn't even attempted if both of the first two statements are false.

Here it is portably w/ case and expr:

case  ${2%oad*}:${1%crypt}  in
(:${key:=$2}*|*ad*|*:"$1")  ;;
(*l:De|*l:En)  expr " $2" :  \
" \(\(.two\)*\(load\)*\)*$" &&
     key=aNXlye1tGbd0uP
esac >/dev/null   

And if:

if   [ Encrypt = "$1" -o Decrypt = "$1" ] &&
     expr " $2" : " \(.two\)*load\(.two\)*$"
then key=$newkey
else key=${key:-$2}
fi   >/dev/null

And just expr and &&||, because the if doesn't do much for you:

expr   > /dev/null       \
 "${#1} ${1%crypt}  $2" :\
 "7 \(\(De\)*\(En\)*\) ""\
 \(.two\)*load\(.two\)*$" &&
key=$newkey || key=${key:-$2}

Perhaps w/ grep:

grep -xqE '(En|De)crypt:(.two)*load(.two)*' \
<<VAR &&   key=$newkey || key=${key:-$2}
${1##*:*}:$2
VAR