Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add warning about shorter if and && #127

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

e40
Copy link

@e40 e40 commented Mar 26, 2022

Definitely nice to use && as a shorter if, but there are caveats,
when using && in a function.

This is probably the single largest error I used to make. It took me far too long to remember this.

Definitely nice to use `&&` as a shorter `if`, but there are caveats,
when using `&&` in a function.
}

func_example2() {
[ $1 -gt 11 ] && return

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this use case one should explicitly supply exit code to return, so that it doesn't use last command's (test) exit status by default. And this is what the difference between the two constructions is. So that's kind of NOT unexpected result. See help return. I think this should be mentioned in this snippet to clarify what the "unexpected result" means, alongside the proper usage.

@bradrf
Copy link

bradrf commented Mar 28, 2022

Another example of why this is a good warning that always bites me is when you're working with scripts that use set -e:

❯ ( set -e ; func_example1 10; echo after ); echo $?
after
0

❯ ( set -e ; func_example2 10; echo after ); echo $?
1

As you can see, because example2 effectively executes a "failed" command, set -e terminates execution, often confusing folks about why when it's in a larger script's context.

@yermulnik
Copy link

yermulnik commented Mar 28, 2022

@bradrf Basically the "correct" function used as an example in this PR is sort of weird since it returns success in both cases despite arg is greater than 11 or not. Meaning that this lacks error handling and hence the "bug". That is why such snippet suggested in this PR should better be supplied with an example of proper use.

E.g. [ $1 -gt 11 ] && return 0 || return 0 in the second ("incorrect") function behaves exactly as the first ("correct") function:

> cat ~/tmp/zzz.sh
#!/usr/bin/env bash

set -e

func_example() {
        [ $1 -gt 11 ] && return 0 || return 0
}

func_example 10 && echo After

> ~/tmp/zzz.sh
After

What I mean is there's no error or improper use, but the lack of proficiency and experience 😉

@georgalis
Copy link

I wanted to update the Important note: using the && syntax in a function can lead to unexpected results text but I've lost track of where it is...

I use &&, ||, brace, and paren exclusively. I never use if/then and I'm familiar with the regular behavior, so when I read "unexpected results" I am confused about what is being pointed out, it's all as expected--as far as I can see. Could there be some more verbosity about what is being demonstrated here? If that is unexpected... I think there is some (not-necessarily true) assumptions about what is expected???

Also regarding option errexit, that can get messy in your shell prompt environment. If you are creating functions for use on your shell prompt, parenthetical functions can be more optimal then the bracketed ones, eg we have the same directory on errexit even if pwd has been changed within the function. Paren functions inherit exported variables, but since it is a subshell, all set variables are basically local. There is a performance hit with paren functions as well.

subshell_function () ( # subshell sets pipefail, errexit and PWD, only within the function
    set -o errexit   # Exit on command non-zero status
    set -o pipefail  # exit pipeline on non-zero status
    set -o errtrace  # any trap on ERR is inherited by shell functions
    set -o functrace # traps on DEBUG and RETURN are inherited by shell functions
    cd /tmp
    false
    true
    )

This function doesn't reach the true statement, it returns status 1, the pwd and original environment are unchanged. However if braces were used, the shell prompt would exit, fix that and PWD would change when the function is called.

@georgalis
Copy link

BTW - this prompt prints the status of prior command, iff it is non-zero, handy!

export PS1="\${?%0} \u@\h:\w "

and here is an unexpected behavior for me :-)

if I set p="-3" then test if the string is set... error. I get the desired result with set p=-3.0

t= p=-5 f=
[ "$t" -o "$p" -o "$f" ] && echo y || echo n
-bash: [: -5: unary operator expected
n
expr "$p" : '^-[[:digit:]]*$' >/dev/null && p="${p}.0"
[ "$t" -o "$p" -o "$f" ] && echo y || echo n
y

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants