mvdan / sh

A shell parser, formatter, and interpreter with bash support; includes shfmt
https://pkg.go.dev/mvdan.cc/sh/v3
BSD 3-Clause "New" or "Revised" License
7.1k stars 336 forks source link

syntax: turn FunctionNextLine into an option that applies to "{", "then", and "do" #721

Open mvdan opened 3 years ago

mvdan commented 3 years ago

See https://github.com/mvdan/sh/issues/229#issuecomment-862734437.

The existing https://pkg.go.dev/mvdan.cc/sh/v3/syntax#FunctionNextLine option is available via shfmt -fn. It works fairly well:

$ echo 'f() { foo; bar; }' | shfmt
f() {
    foo
    bar
}
$ echo 'f() { foo; bar; }' | shfmt -fn
f()
{
    foo
    bar
}

However, it only affects the { opening token, and not others:

$ echo 'for i in 1 2 3; do if true; then foo; bar; fi; baz; done' | shfmt
for i in 1 2 3; do
    if true; then
        foo
        bar
    fi
    baz
done

I propose to replace FunctionNextLine and -fn with a more general option, which would put all opening tokens on a separate line. For example, if we called the flag -tn for TokenNextLine, the behavior would be:

$ shfmt -tn input.sh
f() { foo; } # single-line still permitted
f()
{
    foo
    bar
}

if true; then foo; fi # single-line still permitted
for i in 1 2 3
do
    if true
    then
        foo
        bar
    fi
    baz
done

This could either be implemented in v3, keeping -fn and FunctionNextLine deprecated, or in a future v4 replacing the existing option. The existing option would be removed in v4 either way; the question is whether we should prioritize adding this option sooner, or pushing for v4 instead.

mvdan commented 3 years ago

I should also clarify that this option would apply to all { tokens as well, not just those for functions. For example:

for i in 1 2 3
{ # this newline doesn't break anything
    foo
}

coproc foo { # note that a newline before "{" changes the program here
    bar
}
gaelicWizard commented 3 years ago

fwiw, I'd love to have this available sooner than later 😃

wwalker commented 2 years ago

I, also, would love to have this available sooner than later.

However, I would ask that it be separate.

mvdan commented 2 years ago

However, I would ask that it be separate.

If you mean keep the current -fn flag as-is, and add a new flag for then and do, that's very unlikely to happen. More options make my job harder, the tool more difficult to document and use, and add long-term maintenance and testing work.

So I really only add options when I have to. This disctinction you bring up feels like a minor preference in an opt-in feature, and I don't think there's any style/formatting guide that strongly separates the two cases.

mvdan commented 2 years ago

@gaelicWizard I'm not sure if it's on purpose, but note how you're filling some of the issues here with hundreds of commit references. Please tone it down if you can, as it makes the threads hard to read.

ale5000-git commented 1 year ago

Hi, personally I do want curly brackets of functions on the next line but do and then on the same line. do and then are neither functions nor curly brackets so I think they never belong here.

PS: I vote for the officially listed => curly_bracket_next_line

mvdan commented 1 year ago

It's fine if you want a specific way to format your programs. But shfmt can't support every option and formatting style possible, because that is essentially impossible to maintain and test. Or even use - just see https://linux.die.net/man/1/indent. Most people find this proposal to be a good idea per this thread, and I don't see the point in maintaining multiple options per the above.