StyLua icon indicating copy to clipboard operation
StyLua copied to clipboard

Turns readable multi-line function calls into harder to read, single line ones, but are under the column width

Open Kampfkarren opened this issue 4 years ago • 14 comments

I'm not sure if this is something that's able to be fixed, but this is the before and after. I much prefer the before.

image

image

image

image

vvv This is 140 columns long, without any configuration.

image

Kampfkarren avatar Mar 13 '21 07:03 Kampfkarren

I definitely also prefer the former styling here, so this should be looked into.

I think part of the reason for this problem is because of the way I set up formatting for function args - when it happens, StyLua only sees the parentheses and anything inside of them, not the previous information on the line - so it's hard to judge when to wrap. I'm probably going to have to give it access to some more context

JohnnyMorganz avatar Mar 13 '21 11:03 JohnnyMorganz

Just thought I'd let you know - you may find that the formatting is better for you if you reduce the column with (maybe to something like 80 or 100). It may help in these situations, but there are still cases where it won't do as wanted/expected (such as your local assignment example with three variables - it won't catch that this line is greater than the column width.)

JohnnyMorganz avatar Mar 18 '21 20:03 JohnnyMorganz

Maybe, but 120 cols is already the rule I follow internally, feels weird to lower that down for the sake of StyLua.

Kampfkarren avatar Mar 18 '21 23:03 Kampfkarren

Yeah I ran into this as well, feels a bit weird having this tied to the col width only!

Stanzilla avatar Apr 18 '21 12:04 Stanzilla

I was looking at this further - and I realise that the first four screenshots you posted - they are all under the column width. I can fix the last one, because that's over it, but I'm not sure what to do with the first few ones.

Sticking with column width as a rule, the first few ones are all valid. If I were to paste the same code into prettier (and change column width to 120), prettier would do the same thing here.

I do agree that the pre-formatting versions, they look nicer. But I'm not entirely sure what kind of heuristic to apply here to get something like that, whilst keeping the column width set to 120.

JohnnyMorganz avatar Apr 29 '21 13:04 JohnnyMorganz

Function call which contains long concatenated string arguments are turned into single line. Can this be fixed?

For example

error("11111111111111111111111111111111111111111111111"
 .. "11111111111111111111111111111111111111111111111111111111")

I want this to be 2 lines to be fit in 120 columns, rather than a single line of

error("11111111111111111111111111111111111111111111111" .. "11111111111111111111111111111111111111111111111111111111")

SafeteeWoW avatar May 01 '21 19:05 SafeteeWoW

Function call which contains long concatenated string arguments are turned into single line. Can this be fixed?

For example


error("11111111111111111111111111111111111111111111111"

 .. "11111111111111111111111111111111111111111111111111111111")

I want this to be 2 lines to be fit in 120 columns, rather than a single line of


error("11111111111111111111111111111111111111111111111" .. "11111111111111111111111111111111111111111111111111111111")

I have opened a new issue for this, as it is slightly different to this issue: #139

JohnnyMorganz avatar May 01 '21 19:05 JohnnyMorganz

Demonstrations using column_width

While there's agreement that column_width isn't a solution, I was trying to figure out how to use it and thought I'd document my findings and provide some sample code for testing.

Given some code in the OP:

HealthRegen.ValidateInstance = t.intersection(
	ComponentUtil.HasComponentValidator("Health"),
	ComponentUtil.HasComponentValidator("Target")
)

return function(...)
	callback(
		LOG_FORMAT:format(
			os.date("%H:%M:%S"),
			key,
			level,
			fmt.fmt(...)
		)
	)
end

I have to drop down to --column-width=70 to get the callback to split. At 80, it was still one line.

But at 70 then it can get overly aggressive:

fh:write(n_tests, " tests (", n_passed, " ok, ", n_tests - n_passed, " failed, ", n_errors, " errors)\n")
-- what would be left alone at 120 becomes
fh:write(
    n_tests,
    " tests (",
    n_passed,
    " ok, ",
    n_tests - n_passed,
    " failed, ",
    n_errors,
    " errors)\n"
)

idbrii avatar Jan 16 '22 19:01 idbrii

Seems like the current rule is: if a function call joined to a single line would be over the column limit, then break it on each argument (comma).

Proposal

Change the rule to take the current state into account:

  • if a function call is currently broken after each argument
  • or if a function call joined to a single line is over the column limit
  • then break it on each argument.

However, I'm not sure how you'd handle cases where the input doesn't break on each arg, but only on some args:

fh:write(
    n_tests, " tests (",
    n_passed, " ok, ",
    n_tests - n_passed, " failed, ",
    n_errors, " errors)\n"
)

return function(...)
    callback(
        LOG_FORMAT:format(
            os.date("%H:%M:%S"),
            key, level,
            fmt.fmt(...)
        )
    )
end

I think, only break lines that are over the limit?

Alternatively, handle function calls where the first line ends with a ( differently from ones that end with an arg:

LOG_FORMAT:format(os.date("%H:%M:%S"),
    key, level,
    fmt.fmt(...)
    )
-- becomes (single line under column-width) 
LOG_FORMAT:format(os.date("%H:%M:%S"), key, level, fmt.fmt(...))
LOG_FORMAT:format(
    os.date("%H:%M:%S"),
    key, level,
    fmt.fmt(...)
    )
-- becomes
LOG_FORMAT:format(
    os.date("%H:%M:%S"),
    key,
    level,
    fmt.fmt(...)
)

But a function that when single-lined would be beyond column-width always becomes the second style.

idbrii avatar Jan 16 '22 19:01 idbrii

Change the rule to take the current state into account:

  • if a function call is currently broken after each argument

I'm a bit wary about relying on existing input state like this. It leads to a few issues, and is essentially a "hidden" option. We do have something similar for tables, but it also has the same formatter reversibility issue (also described by prettier https://prettier.io/docs/en/rationale.html#%EF%B8%8F-a-note-on-formatting-reversibility):

What does reversible mean? Once an object literal becomes multiline, Prettier won’t collapse it back. If in Prettier-formatted code, we add a property to an object literal, run Prettier, then change our mind, remove the added property, and then run Prettier again, we might end up with a formatting not identical to the initial one. This useless change might even get included in a commit, which is exactly the kind of situation Prettier was created to prevent.

If we rely on the input state (looking for a newline between the first brace and argument), then if you add another argument which goes over the column width, it gets expanded. If you then delete this argument (where it would now be under the column width), you are still expanded, and always will be until you explicitly remove the newline. This reversibility issue is a problem (which can lead to unnecessary diffs too), and something which we want to try and avoid.

JohnnyMorganz avatar Jan 17 '22 15:01 JohnnyMorganz

Feels like this issue is "humans know better how to make this understandable" running up against purely syntactic formatting. (Similarly, I'd prefer hidden options to adding --stylua: ignore annotations.) Regardless, stylua's opinionated stance is a design goal so I can see how more options deviates from that goal.

idbrii avatar Jan 18 '22 23:01 idbrii

For the input below, I expected that the formatted output kept the same amount of lines:

foo.bar('-------------------------------------------------------------------------------------------------------------')
	.returns()
foo.bar('--------------------------------------------------------------------------------------------------')
	.returns()
	.way()
	.longer()
	.chain()

However, StyLua formatted it like the output below, where each function call chain is joined on a line. The lines are longer than the column limit of 120, and that doesn't read nice.

foo.bar("-------------------------------------------------------------------------------------------------------------").returns()
foo.bar("--------------------------------------------------------------------------------------------------").returns().way().longer().chain()

I'm posting it here, because it looks like a duplicate of this issue.

TjeuKayim avatar Feb 09 '22 10:02 TjeuKayim

I run into this issue recently. Could you please just add an option to keep line breaks (for non-empty lines)? This way users can fix the case manually.

afknst avatar Feb 13 '22 16:02 afknst

I have some code like bellow;

-- A
a = A()

-- B
.B()

-- C
.C()

And after several runs, the formatter makes it like this:

-- A
a = A() -- B.B() -- C.C()

This is intolerable since the code does totally different things after being formatted.

afknst avatar Feb 13 '22 17:02 afknst