Indentation stripped from elif statements

[mio-bryan]

Hello, I seem to have encountered an issue which seems to be the inverse of #2

With the given block of code (tabs or spaces):

#!/usr/bin/env bash

foo(){
    thing=${1}
    if [[ ${thing} == "bob" ]]
    then
        echo "Yar!"
    elif [[ ${thing} == "jack" ]]
    then
        echo "Yip!"
    else
        echo "Nar"
    fi
}

for thing in "bob" "jim" "jack"
do
    foo "${thing}"
    if (( $? == 0 ))
    then
        echo "Returned success"
    elif (( $? > 0 ))
    then
        echo "No bueno"
    else
        echo "Bad magic"
    fi
done

When run through beautysh ( 4.1 or 5.0.2 ), indentation is stripped at elif:

$ beautysh -v
5.0.2
$ beautysh foo.sh
$ cat foo.sh 
#!/usr/bin/env bash

foo(){
    thing=${1}
    if [[ ${thing} == "bob" ]]
    then
        echo "Yar!"
elif [[ ${thing} == "jack" ]]
    then
        echo "Yip!"
    else
        echo "Nar"
    fi
}

for thing in "bob" "jim" "jack"
do
    foo "${thing}"
    if (( $? == 0 ))
    then
        echo "Returned success"
elif (( $? > 0 ))
    then
        echo "No bueno"
    else
        echo "Bad magic"
    fi
done

[mio-bryan]

Able to replicate this with docker and ubuntu eoan, python3 and beautysh 6.0.1

cat > test.sh
#!/usr/bin/env bash

while true
do
    if (( $? == 0 ))
    then
        echo "Returned success"
    elif (( $? > 0 ))
    then
        echo "No bueno"
    else
        echo "Bad magic"
    fi
done

cat > Dockerfile
FROM ubuntu:eoan
ENV LANG en_US.utf8
RUN apt update && apt -y upgrade && apt install -y python3-pip
RUN pip3 install beautysh
COPY test.sh /test.sh
RUN beautysh -b /test.sh

docker build .
...
docker run <container_id> diff /test.sh /test.sh.bak
8c8
< elif (( $? > 0 ))
---
>     elif (( $? > 0 ))

docker run <container_id> beautysh --version
6.0.1

[trystero11]

Replicated with beautysh 6.0.1.

[trystero11]

It seems to matter whether exactly where the elif statement is located. Here's my test Bash script (adapted from #2 and the test that @mio-bryan posted above), with if in a function declaration, inside a for loop, at the top level with then on the same line as if, and at the top level with then on its own line:

#!/usr/bin/env bash

foo(){
    thing=${1}
    if [[ ${thing} == "bob" ]]
    then
        echo "Yar!"
    elif [[ ${thing} == "jack" ]]
    then
        echo "Yip!"
    else
        echo "Nar"
    fi
}

for thing in "bob" "jim" "jack"
do
    foo "${thing}"
    if (( $? == 0 ))
    then
        echo "Returned success"
    elif (( $? > 0 ))
    then
        echo "No bueno"
    else
        echo "Bad magic"
    fi
done

if [ -f testfile1 ]; then
    echo "1"
elif [ -f testfile2 ]; then
    echo "2"
else
    echo "3"
fi

if [ -f testfile1 ]
then
    echo "1"
elif [ -f testfile2 ]
then
    echo "2"
else
    echo "3"
fi

Run on this script, beautysh 6.0.1 incorrectly outdents the elif statements in the function declaration and in the if inside the for loop:

beautysh -b test.sh && diff test.sh.bak test.sh
8c8
<     elif [[ ${thing} == "jack" ]]
---
> elif [[ ${thing} == "jack" ]]
22c22
<     elif (( $? > 0 ))
---
> elif (( $? > 0 ))

If I strip elif out of the regular expression on line 236 of beautysh.py, it correctly indents the elif statements in the function declaration, the if statement inside the for loop, and the top-level if statement with then on its own line, but incorrectly indents the one in the top-level if statement with then on the same line as if:

beautysh -b test.sh && diff test.sh.bak test.sh
32c32
< elif [ -f testfile2 ]; then
---
>     elif [ -f testfile2 ]; then

[lovesegfault]

I wonder if we just want to enforce the linebreak between if/elif and then. Could be a fine way to solve this IMO.

Fundamentally, the regex-based approach beautysh takes is insufficient to correctly handle all of bash. If I were to rewrite it today I would parse the bash script with bashlex and then applied formatting rules to that.

[trystero11]

I think we can fix this by making the ad-hoc outdent only apply to elif when it's followed by then on the same line. Testing with that change indents all four elif statements in my test script correctly. I'll submit a PR for review.

[trystero11]

Looks as though the actual issue is that lines containing then get indented once (line 203), while lines containing elif get outdented twice (lines 206 and 236/243).

So if you have elif and then on the same line, as in the test case for #2, you end up with a net one-step outdent, which is correct. But if you have elif on a line without then, as in the test case for this issue, you get a net two-step outdent, which isn't.

By making the second (ad-hoc) elif outdent conditional on finding then on the same line, I think we should get the right behavior for everyone, regardless of whether they prefer the one- or two-line form.

[lovesegfault]

Fixed in #75 thanks to @trystero11!