Add option to preserve newline gaps for blocks

[InoUno]

Adds an option that allows preservation of newline gaps at the start and/or end of blocks - see example at the end of PR.

This is similar to how newline gaps are already preserved between statements. Namely that the existence of newline gaps between statements is also dependent on if there's a gap in the input string:

local x = 1 -- the below gap is preserved

local y = 2

I'm aware that there's discussion about if more config should be added in #620, so this PR can serve as another option suggestion.

Adding this option solves this issue, but in a more general way for all blocks, instead of just for if-else-if chains.

This could instead be implemented on a subset of newline gaps (i.e. only for leading newline gaps before else-if/else tokens), or even enforced always with a different option, as suggested in the above issue.
However, I went with the more general solution in this PR, since notably gofmt also preserves newline gaps at the start and end of blocks in this way as well. It is also a quite opiniated formatter, but doesn't have an opinion on those gaps (similar to statement gaps), besides them being at most a 1-newline gap.

Motivation and example

In our codebase, we occasionally have some long if-else-if chains, and they become quite hard to read when there's no newline gaps allowed in them. This is especially true when the condition itself is multiline, since it then becomes indented at the same level as the inner block statements:

if
    this_is == very_long_variable_name
    and to_ensure_that == it_is_broken_into
    and multiple_lines == in_order_to_see_how_that_looks
then
    do_this_thing = 0
elseif
    this_is == very_long_variable_name
    and to_ensure_that == it_is_broken_into
    and multiple_lines == in_order_to_see_how_that_looks
then
    do_something_here = 1
    with_a_few_long_lines_as_well = 2
elseif
    this_is == very_long_variable_name
    and to_ensure_that == it_is_broken_into
    and multiple_lines == in_order_to_see_how_that_looks
then
    do_something_else_here = 3
    with_a_few_long_lines_as_well = 4
else
    return 1
end

Having newline gaps makes it a bit easier to distinguish where the different if-blocks start and end:

if
    this_is == very_long_variable_name
    and to_ensure_that == it_is_broken_into
    and multiple_lines == in_order_to_see_how_that_looks
then
    do_this_thing = 0

elseif
    this_is == very_long_variable_name
    and to_ensure_that == it_is_broken_into
    and multiple_lines == in_order_to_see_how_that_looks
then
    do_something_here = 1
    with_a_few_long_lines_as_well = 2

elseif
    this_is == very_long_variable_name
    and to_ensure_that == it_is_broken_into
    and multiple_lines == in_order_to_see_how_that_looks
then
    do_something_else_here = 3
    with_a_few_long_lines_as_well = 4

else
    return 1
end

[InoUno]

@JohnnyMorganz Is adding this option something that's on the table now that #620 is decided? If so, I'll look into fixing the recently emerged conflicts.

[JohnnyMorganz]

Sorry, I never got round to looking at this PR. This one is interesting, I didn't realise gofmt also preserved it like this, thanks for that link.

I'm not a big fan on options to 'preserve' syntax (although we do have options to do this already, like for function call parentheses, since that was a Hot Topic). Mainly because it makes it more difficult to apply consistent reproducible formatting without human intervention. This reopens the door of stylistic nit comments in e.g. code review - now if you accidently leave a newline at the end, you have to manually go in and remove it. Ideally the formatter should take a block and format it consistently (with a set of configurable options to tweak how it can look that are decided ahead of time and stored in a config file, not decided per-code-review).

But, to be fair, gating it behind an option does alleviate most of that concern - because you'll have to make an explicit decision to opt-in to this behaviour.

Before committing to this most general solution where lines are determined ad-hoc, what do you think about the alternative option to always add a newline at the start / end of a block? Or would that be overly verbose in some cases?

[InoUno]

I'm not a big fan on options to 'preserve' syntax (although we do have options to do this already, like for function call parentheses, since that was a Hot Topic). [...] Ideally the formatter should take a block and format it consistently [...]

I generally agree that this would be ideal, but I don't think it's practical in all cases, because readability of code is sometimes determined by the content of the code.

I see these newline gaps for start/end of blocks to be very similar to newline gaps between statements, which most formatters preserve at least 1 of (StyLua included currently). The existence of these gaps depends on the semantics/meaning of the code for how they fit in to split up chunks of code, and it can't be determined from just the AST (without some crazy semantic analysis). Small example:

-- Configure a
a.x = 1
a.y = 2

-- Configure b
b.x = 3
b.y = 4

Before committing to this most general solution where lines are determined ad-hoc, what do you think about the alternative option to always add a newline at the start / end of a block? Or would that be overly verbose in some cases?

I personally think that would be overly verbose, if it's done for both start and end for all blocks.
But instead of doing this as an alternative, what about just adding these 3 extra cases as options?

So the options would be like:

pub enum BlockNewlineGaps {
    /// Never allow leading or trailing newline gaps for blocks
    #[default]
    Never,
    /// Preserve both leading and trailing newline gaps for blocks, if present in input
    PreserveBoth,
    /// Preserve leading newline gaps for blocks, if present in input
    PreserveLeading,
    /// Preserve trailing newline gaps for blocks, if present in input
    PreserveTrailing,
    /// Always add both leading and trailing newline gaps for blocks
    AlwaysBoth,
    /// Always add a leading newline gaps for blocks
    AlwaysLeading,
    /// Always add trailing newline gaps for blocks
    AlwaysTrailing,
}

[InoUno]

@JohnnyMorganz Sorry to bump, but just making sure it's not going to go unnoticed again. Do you have any further thoughts/conclusions on this?

[JohnnyMorganz]

Very sorry for the lack of response here. Finally got some time to take a look at stylua again.

I'm happy to commit to this. Agreed that maybe mixing always + preserve options is a bit too much for now. We don't need to do that yet, but lets design the config to keep the door open for that in the future if necessary.

Do we need the separate options for just leading / just trailing? Or would just having 2 options (Never and Always) be sufficient?

Not sure what the best name for the config / options is. How about block_newline_gaps with options Never and PreserveAlways? We can add the leading / trailing separately if needed.

Not sure if block_newline_gaps covers the fact that we are only talking about leading / trailing gaps though, but block_leading_trailing_newline_gaps might be a bit too verbose...

[InoUno]

Very sorry for the lack of response here. Finally got some time to take a look at stylua again.

I'm happy to commit to this. Agreed that maybe mixing always + preserve options is a bit too much for now. We don't need to do that yet, but lets design the config to keep the door open for that in the future if necessary.

Do we need the separate options for just leading / just trailing? Or would just having 2 options (Never and Always) be sufficient?

Not sure what the best name for the config / options is. How about block_newline_gaps with options Never and PreserveAlways? We can add the leading / trailing separately if needed.

Not sure if block_newline_gaps covers the fact that we are only talking about leading / trailing gaps though, but block_leading_trailing_newline_gaps might be a bit too verbose...

Happy to hear!

I agree and think that block_newline_gaps and options Never and Preserve is fine, and then a doc/comment can describe that it's for leading and trailing gaps of blocks.

I've updated the PR to fix the conflicts and change to the new option naming.

[InoUno]

Sorry to bump again, @JohnnyMorganz (it's been a while). Is there a chance this can get looked at/added? We're currently waiting for it, before we start formatting everything in our project with StyLua.

[JohnnyMorganz]

Super super sorry for the delay once again, it is difficult to find the time.

Implementation looks pretty simple. I'm happy to move forward with this.

Thank you for your patience :)