Giter VIP home page Giter VIP logo

Comments (7)

mio-bryan avatar mio-bryan commented on June 21, 2024

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

from beautysh.

trystero11 avatar trystero11 commented on June 21, 2024

Replicated with beautysh 6.0.1.

from beautysh.

trystero11 avatar trystero11 commented on June 21, 2024

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

from beautysh.

lovesegfault avatar lovesegfault commented on June 21, 2024

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.

from beautysh.

trystero11 avatar trystero11 commented on June 21, 2024

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.

from beautysh.

trystero11 avatar trystero11 commented on June 21, 2024

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.

from beautysh.

lovesegfault avatar lovesegfault commented on June 21, 2024

Fixed in #75 thanks to @trystero11!

from beautysh.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.