syntax: case clause formatting can add empty line when removing a newline

[ainar-g]

Before:

case "$x"
in
(0)
        echo 'none!'
        ;;
(1)
        echo 'it'"'"'s one!'
        ;;
(*)
        echo 'something!'
        ;;
esac

After shfmt -p -s:

#!/bin/sh

set -e -f -u -x

x="${1:-0}"
readonly x

case "$x" in

0)
        echo 'none!'
        ;;
1)
        echo 'it'"'"'s one!'
        ;;
*)
        echo 'something!'
        ;;
esac

POSIX allows case items to start with a parenthesis, and I prefer it, because it makes case look more understandable, imo. I would like an option to either add them or at least not remove them.

(Also, I've noticed that shfmt seems to add an empty line after case … in, which looks like a bug.)

[mvdan]

(Also, I've noticed that shfmt seems to add an empty line after case … in, which looks like a bug.)

That sounds like a bug indeed :)

an option to either add them

This feels like such a tiny detail to warrant adding a formatting flag. As you're aware from gofmt, the more flags the formatter gains, the less useful it becomes. But also, the more difficult it is for me to explain and document. Some instances like indentation have meet the bar for importance, but I don't think this case meets that bar.

Note that we already have a flag for formatting switches, -ci, though it's about the indentation. I don't think that's related to the left parenthesis in any way.

at least not remove them.

I'm not sure. Part of the tool's design is to produce consistent output wherever possible. If we simply obeyed the user's choices everywhere, we wouldn't really format anything.

For example, what if one switch mixes both styles? Should we enforce just one? What if one file has multiple switches that each use different styles? At least with the current logic, the output is consistently formatted.

[ainar-g]

All valid points, thanks!

Should I close the issue, or are you planning on fixing the empty line bug?

[mvdan]

We should fix the empty line bug, yup!

[Julien-Elie]

Hi Daniel, keeping the leading parenthesis in (0) is what the pre-posix dialect is expected to do. The intent of the proposal in #757 is exactly for that: doing no syntax changes at all (preserve what the user wrote), just applying reformatting rules (indentation, alignment, ...) without changing the code.

[mvdan]

I'm not sure I follow; "use syntax that works on pre-POSIX shells" is different from "keep the existing syntax as-is".

[Julien-Elie]

Oh yes, you're right.
I kinda mix these two meanings as the only syntax change I noticed in the scripts I reformatted was these backticks.
So maybe there could be a need for both "dialects": pre-posix (that could change syntax, if working on pre-POSIX shell, but how do you know 0) works on all of them, and (0) is not expected on one of such pre-POSIX shells?) and keep-syntax (that would not change any syntax)?
Maybe it would make happy other people who want to keep their preferred syntax? (like in this issue)

[mvdan]

I'm still fine with adding pre-posix. I disagree with adding keep-syntax; it's not a language dialect, and even as an option of its own, it goes out of the way to disable a lot of the formatter's features. We'd have to add code and complexity just for the sake of forcing the formatter to only change indentation and whitespace.

So, all in all, it doesn't seem worthwhile. I've tried options of the form "keep the user's syntax" before, such as #658, and it's not worth it.

[mvdan]

I did not mean to hit "close and comment" :)

[Julien-Elie]

OK, no problem, I understand your point. I was essentially suggesting it because some other widely-used reformatters do not change the syntax but only improve readability (like perltidy or clang-format, except for #include re-ordering but it can be disabled).

[Julien-Elie]

The empty line is also wrongly kept in statements (not only after case...in):

case "$x" in
a)
  ;;
b) ;;
esac

is reformatted:

case "$x" in
a) ;;

b) ;;
esac