StyLua
StyLua copied to clipboard
Optional parentheses: keep on foo('bar')(baz)
In line with https://github.com/JohnnyMorganz/StyLua/issues/133#issuecomment-846131062, here is one more special case where one might wish to keep parentheses even with no_call_parentheses
: If the function is a "factory" that returns another function that is called in place; I don't remember where I saw it in the wild, but it was something like
require('foo.bar')(baz)
which then gets turned into
require 'foo.bar'(baz)
which is syntactically correct but might look ambiguous? (The corresponding foo { bar = 'baz' }(quux)
is fine, though.)
Hm, personally I do agree that I prefer the former style compared to the latter. My main concern right now is how much of an impact it will have on codebases now given how much more adoption stylua has had, and if everybody else would also agree with the style change. The latter is impossible to determine, since most don't track the stylua issues.
Sure, I just wanted to put it out there; it's not a pressing matter from my side. Let's wait and see whether the ayes will have it :)
I'm definitely more a fan of removing the parentheses. This is heavily used in the soon-to-release UI framework by Elttob. Example:
New "TextLabel" {
BackgroundTransparency = 0.5
}
This would look a lot worse with this change
New("TextLabel"){
BackgroundTransparency = 0.5
}
or even worse
New("TextLabel")({
BackgroundTransparency = 0.5
})
Especially with double quotes, the default in StyLua, the first of these 3 is clearly readable, instead of being unnecessarily cluttered.
@Dionysusnu definitely a valid point. I know also that sumneko's lua language server (not that its formatted with stylua, but just as an example) uses a custom DSL like
grammar "name" [[
foo bar
]]
which is similar to what you mention.
What about, alternatively, we keep parentheses when the next chain call takes arguments where parentheses must remain (i.e. whats shown in OP), but we remove them if the next chain call takes a single string/table argument (i.e. where parentheses can be removed)
Input | New Formatting |
---|---|
require("foo")(bar) or require "foo"(bar) |
require("foo")(bar) |
require(foo)("bar") or require(foo)"bar" |
require(foo)("bar") |
require("foo")("bar") or require "foo" "bar" |
require "foo" "bar" |
require("foo")({ ... }) or require "foo" { ... } |
require "foo" { ... } |
This definitely adds a bit more complexity (both in impl + understanding how stylua formats), but should be workable. This may be the best of both worlds?
What about, alternatively, we keep parentheses when the next chain call takes arguments where parentheses must remain.
That describes precisely the situation I had in mind, which I wanted to illustrate with an example.
How would this interact with the rule from https://github.com/JohnnyMorganz/StyLua/issues/133#issuecomment-846131062?
I prefer require'foo.bar'(baz)
(without unnecessary space), this visually focuses on the relevant function call.
@Dionysusnu definitely a valid point. I know also that sumneko's lua language server (not that its formatted with stylua, but just as an example) uses a custom DSL like
grammar "name" [[ foo bar ]]
which is similar to what you mention.
What about, alternatively, we keep parentheses when the next chain call takes arguments where parentheses must remain (i.e. whats shown in OP), but we remove them if the next chain call takes a single string/table argument (i.e. where parentheses can be removed)
Input New Formatting
require("foo")(bar)
orrequire "foo"(bar)
require("foo")(bar)
require(foo)("bar")
orrequire(foo)"bar"
require(foo)("bar")
require("foo")("bar")
orrequire "foo" "bar"
require "foo" "bar"
require("foo")({ ... })
orrequire "foo" { ... }
require "foo" { ... }
This definitely adds a bit more complexity (both in impl + understanding how stylua formats), but should be workable. This may be the best of both worlds?
I think this should be a separate option. For example, in the Fusion codebase, there are a lot of cases of Instance.new("Folder")
, which we want to keep with parentheses. At the same time, we want to remove the parentheses on the curried calls, like New "TextLabel" { ... }
Or, more general, it should be configurable for tables and strings separately. Strings look bad when not in parentheses in non-curried calls, but tables look fine.
I think this should be a separate option. For example, in the Fusion codebase, there are a lot of cases of
Instance.new("Folder")
, which we want to keep with parentheses. At the same time, we want to remove the parentheses on the curried calls, likeNew "TextLabel" { ... }
Or, more general, it should be configurable for tables and strings separately. Strings look bad when not in parentheses in non-curried calls, but tables look fine.
If I were to add an option here, I'm not sure how it would solve either of the two things you mention.
How would you describe an option which kept Instance.new("Folder")
parentheses, but left it for New "TextLabel" { ... }
- they both have string arguments. To pick another choice from fusion, there's also OnEvent "Activated"
- only a single string argument, like Instance.new, but with parentheses removed. It's pretty much impossible to have an option suiting all these cases here.
Even then, actually adding a new option is currently unlikely to happen. (However, I was discussing with some others a while ago about scrapping the whole "opinionated, non-configurable" nature of StyLua and becoming completely customisable, like rustfmt. It simplifies having to deny people's style ideals, but not sure if this is a route I want to go down - going to think about it. For others who are interested in something like that right now, I know LuaFormatter exists)
Strings look bad when not in parentheses in non-curried calls, but tables look fine.
Thats your personal preference here, but others may be the complete opposite 😄
On the topic of the current issue pointed out by OP, I still haven't really decided on the style to go for. I know bfredl pointed out another way to format - its hard to please everyone! If people have any other suggestions, feel free to pop them in
To clarify, my suggestion is:
- Separate options for string and table versions
- A third mode, instead of the true/false, which only does this for curried functions. Although, I agree with what you said, it'll be hard to make a one-size-fits-all here.
Maybe the best option is to add a catch-all "don't touch" option. Although that wouldn't exactly line up with the opinionated route that StyLua tries to take.
I've been thinking about this, and I'm not sure that I want one or the other all of the time. A lot of the time I choose to omit parentheses because of the surrounding code context, not using some strict rule.
For example, in Fusion, I omit parentheses from [OnEvent "Activated"] = ...
because I find that to be less visually noisy and slightly easier to read/write than [OnEvent("Activated")] = ...
. Similar logic applies for the New function.
However, I wouldn't omit the brackets on a single print()
call sitting alone on a line, because that's already easy to read.
I'm wondering if it's worth having an option to leave them as-is and instead leave this question of which to use to be answered by style guides and coding conventions for individual projects.
I'm wondering if it's worth having an option to leave them as-is and instead leave this question of which to use to be answered by style guides and coding conventions for individual projects.
The intention of using an automated formatter is to try and remove the need of individuals enforcing code style across a project in the first place. If we rely on leaving it, then we are back to style nit comments in PRs, which is the exact thing a tool like this is trying to avoid.
I think this should be a last resort situation, I would personally want to try and have something as automated as possible first. But indeed as everyone has pointed out, this is quite a difficult thing to solve and suit everyone.
Just wanted to point out (for anyone following this) about the addition in #330 over more granular control over when parentheses are removed. It allows to optionally only remove parentheses around single table arguments, and keep it around string arguments.
I know its not entirely the fix for this problem, but it does solve the original issue in OP by just no longer removing parentheses around string arguments at all, so it may help some people here.
Due to the difficulty in finding a "one-size-fits-all" solution for this issue, I have decided to follow through with #668 and add an "Input"
option where parentheses are preserved based on the input code.
Personally, its not an option I would recommend, but since we already have different options for call parentheses, I think its ok to add another option here to help cater more people. Enabling the option will remove any automation decisions and StyLua will not enforce consistency in call parentheses