Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function calls not hanging when over column width #368

Closed
JohnnyMorganz opened this issue Feb 13, 2022 · 7 comments · Fixed by #470
Closed

Function calls not hanging when over column width #368

JohnnyMorganz opened this issue Feb 13, 2022 · 7 comments · Fixed by #470
Labels
bug Something isn't working

Comments

@JohnnyMorganz
Copy link
Owner

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.

Originally posted by @TjeuKayim in #78 (comment)

@JohnnyMorganz JohnnyMorganz added the bug Something isn't working label Feb 13, 2022
@matthargett
Copy link

matthargett commented Feb 26, 2022

Another angle, I'd expect this to act the same as the examples above:

foo.bar(--[[-------------------------------------------------------------------------------------------------------------]])
	.returns()
foo
	.returns()
	.bar(--[[--------------------------------------------------------------------------------------------------]])
	.way()
	.longer()
	.chain()```

@TjeuKayim
Copy link

Another issue like this:

foo
    .bar('------------------------------------------------------')
    .returns('-----------------------------------------') -- some comment

This is wrapped on one line and then it exceeds the column limit because of the comment.

@JohnnyMorganz
Copy link
Owner Author

JohnnyMorganz commented Mar 3, 2022

Out of interest, what kind of context do you have which chains function calls like this, rather than chained method calls (:foo())?

When I originally implemented call hanging, I assumed that dot index .foo() calls would not be chained, but I guess that's not true. Maybe I should just always hang function calls like we do method calls in all situations?

@TjeuKayim
Copy link

The unit testing framework https://olivinelabs.com/busted/ uses chained function instead of method calls.

assert.spy(s).was.called_with(match._)
foo.on_call_with(1, 2, 3).returns("foo", "bar")

@JohnnyMorganz
Copy link
Owner Author

I'm thinking of making .foo() calls always hang just like :foo() method calls do, rather than only hang if they specifically go over width. This would mean .foo() and :foo() formatting will be the same going forward.

I'm wondering if there are any disadvantages for doing this - do people prefer dot index calls to stay inlined?

Once noticable disadvantage of doing it would be for something like

Promise.new(function(resolve, reject)
    local foo = "bar"
    resolve(foo)
end)
    :andThen(function()
        local x = 1
        local foo = "bar"
        return true
    end)

it would now turn into

Promise
    .new(function(resolve, reject)
        local foo = "bar"
        resolve(foo)
    end)
    :andThen(function()
        local x = 1
        local foo = "bar"
        return true
    end)

But is this actually a disadvantage...? The alignment actually makes it look better imho.

A way to resolve this could be to special case the .new syntax, just like prettier does. We looked into this before in #123 though, and I'm not sure I 100% like such a special casing (especially with something like #123 (comment), which is similar to above)

@matthargett
Copy link

I agree on special-casing new not being great. I think I like the consistency of the hanging more as well, even in this builder pattern case.

@JohnnyMorganz
Copy link
Owner Author

Incredibly sorry about the delay in sorting this out. Should be fixed in #470 and available in the next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants