sh icon indicating copy to clipboard operation
sh copied to clipboard

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

Open mvdan opened this issue 4 years ago • 7 comments

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 avatar Aug 16 '21 12:08 mvdan

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
}

mvdan avatar Aug 16 '21 12:08 mvdan

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

gaelicWizard avatar Aug 22 '21 20:08 gaelicWizard

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

However, I would ask that it be separate.

  • -fn still for {
  • -i-dislike-semicolons-in-if-and-while-and-until for ; then and ; do
    • might want a shorter option name :-)

wwalker avatar Aug 30 '21 18:08 wwalker

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 avatar Aug 31 '21 12:08 mvdan

@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.

mvdan avatar Mar 29 '22 22:03 mvdan

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

ale5000-git avatar Oct 18 '22 09:10 ale5000-git

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.

mvdan avatar Oct 20 '22 13:10 mvdan