Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parse: does not understand case fallthrough tokens #13

Closed
nshalman opened this issue Jun 15, 2016 · 9 comments · Fixed by NixOS/nixpkgs#16272
Closed

parse: does not understand case fallthrough tokens #13

nshalman opened this issue Jun 15, 2016 · 9 comments · Fixed by NixOS/nixpkgs#16272

Comments

@nshalman
Copy link

shfmt is totally fine with this code:

#!/bin/bash

do_fallthrough() {

        case $1 in
                'foo')
                        echo foo
                        ;;
                'bar')
                        echo bar
                        ;;
                *)
                        echo baz
                        ;;
        esac
}

echo
do_fallthrough foo

echo
do_fallthrough bar

echo
do_fallthrough blah

But with this code:

#!/bin/bash

do_fallthrough() {

        case $1 in
                'foo')
                        echo foo
                        ;;
                'bar')
                        echo bar
                        ;&
                *)
                        echo baz
                        ;;
        esac
}

echo
do_fallthrough foo

echo
do_fallthrough bar

echo
do_fallthrough blah

I get this (I believe) erroneous error:
sample.sh:11:4: ; can only immediately follow a statement

Relevant Documentation:
https://www.gnu.org/software/bash/manual/bashref.html#index-case

@mvdan
Copy link
Owner

mvdan commented Jun 15, 2016

Ah, this is simply a bash token (not POSIX) that I hadn't added yet. Thanks for spotting!

@mvdan mvdan changed the title shfmt doesn't understand ";&" fallthrough notation for case statements in bash parse: does not understand case fallthrough tokens Jun 15, 2016
@nshalman
Copy link
Author

No problem. Thanks for sharing this code. Aside from that one token it was perfectly happy to apply a consistent style to a bunch of organically grown scripts that each contained multiple different styles.

If I were a stronger Go programmer and had more time I'd have tried to fix it myself and open a PR, but I figured a good bug report would still be appreciated.

Again, thanks for sharing your code!

@mvdan
Copy link
Owner

mvdan commented Jun 15, 2016

No problem. Thanks for sharing this code. Aside from that one token it was perfectly happy to apply a consistent style to a bunch of organically grown scripts that each contained multiple different styles.

Cool :)

If I were a stronger Go programmer and had more time I'd have tried to fix it myself and open a PR, but I figured a good bug report would still be appreciated.

Parsers become stupidly complex very easily, especially one that parses Shell. So no worries on that front - it's much easier for me to fix things like these as I'm already familiar with the codebase.

@mvdan mvdan closed this as completed in 8add007 Jun 16, 2016
@nshalman
Copy link
Author

Thanks for the quick fix! The new version no longer throws the error for me.
I packaged this tool for NixOS yesterday (NixOS/nixpkgs#16247) and will be updating the package to point to this code.

nshalman added a commit to cerana/nixpkgs that referenced this issue Jun 16, 2016
Among other improvements the author fixed mvdan/sh#13
domenkozar pushed a commit to NixOS/nixpkgs that referenced this issue Jun 16, 2016
Among other improvements the author fixed mvdan/sh#13
@mvdan
Copy link
Owner

mvdan commented Oct 13, 2016

@nshalman could you please update it to 0.2.0 there? Thanks!

@nshalman
Copy link
Author

nshalman commented Oct 13, 2016

As shown above, I've created NixOS/nixpkgs#19524
Thanks for the reminder!
edit: It's been merged.

@mvdan
Copy link
Owner

mvdan commented Oct 14, 2016

@nshalman thanks!

@mvdan
Copy link
Owner

mvdan commented Jan 5, 2017

@nshalman please update to 1.1.0 :)

@nshalman
Copy link
Author

nshalman commented Jan 5, 2017

PR created: NixOS/nixpkgs#21695
edit: merged.

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

Successfully merging a pull request may close this issue.

2 participants