-
Notifications
You must be signed in to change notification settings - Fork 79
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
Allow single line function definitions #248
Comments
Hm, this does seem like a reasonable request. I'm going to think about it, and its open for discussion |
To add onto this further, it seems that prettier does do something similar for "simple" functions: let foo = func(() => true, 2) However, like in #161 for if statements, JavaScript/TypeScript has two different syntaxes to differentiate between these two (with braces, and without braces). let foo = func(() => {
return true;
}, 2); Since, in Lua, we have no differentiation between these two, we need to go all in for either one or the other style (same as with if statements). There are disagreements between which style is better here. |
I understand that adding another setting is a slippery slope, but this behavior can be controlled by something like 'collapse_simple_statements'. When set to |
I'm curious what prettier does for
I'm also curious if the formatting above has more to do with the JS-mandatory |
Verified with prettier 2.32: let foo = func(() => {}, 2);
let foo = func(() => {
return false;
}, 2);
let foo = func(() => false, 2);
let foo = func(() => [true, false, "boo"], 2); what I'd expect the equivalent Lua 'native' code to do: local foo = func(function() end, 2)
local foo2 = func(function() return false end, 2)
local foo3 = func(function() return false end, 2)
local foo4 = func(
function() return true, false, "boo" end,
2
) |
I have a question about the fourth one. Do you expect it not to be one line because function returns multiple values? Because in one line it occupies 62 characters which should pass default width: local foo4 = func(function() return true, false, "boo" end, 2) Also a more Lua 'native' code might be: local foo4 = func(function() return {true, false, "boo"} end, 2) |
Apologies for the long delay on any update for this. Rodux.Store.new(function(state) return state, mockState end, nil)
Rodux.Store.new(function(state) return state end, mockState, nil) On a single line, its much more difficult to realise that there is an issue here (the second version is correct, but if you imagine this in a larger codebase, and you glance over it, this isn't going to be immediately noticed). In comparison with the second expanded version: Rodux.Store.new(function(state)
return state
end, mockState, nil)
Rodux.Store.new(function(state)
return state, mockState
end, nil) Now we can better see that we have a big difference here. If we only applied this to functions with single returns, it isn't really better since we could have a case where version 1 above was actually the correct syntax, but we write version 2 - its just the same issue but inverted. Because of this, I'm inclined to unfortunately say no to this |
Sure, I see where you are coming from. I still think that this might be a good candidate for a setting, as this edge case seems to be quite rare compared to overall benefits (smaller code base; more unified formatting when supplying anonymous function as argument). Strangely enough, those two cases in one line form are immediately seen as different in your example here thanks to syntax highlighting (which is very common, I'd say). |
Haha yeah, as I was writing it I was thinking this too 😄, but normally (for this example) you wouldn't have these two lines next to each other in a real world codebase (probably even just only have one in a single [group of] file[s]), so the syntax highlighting difference wouldn't be visible. I can still consider an option, but unfortunately I am very stubborn on adding new options, so it could be a while until anything comes about from it :P |
Sure, totally get that. Just saying that there will be a demand for this in Neovim world. |
I would like such an option, too, since there are an increasing number of situations in Neovim where you want to pass a function reference with arguments, which requires something like function()
require'foo'.bar( { baz = 'quux' })
end This would go well together with #232, which covers the same (single statement) case for conditionals. As in #330, this could be combined into a single option, e.g.,
(with the first being default). |
I am very sorry to complain again about this, but considered this might be a useful observation on the matter. During writing code I quite often find myself adding --stylua: ignore
func(function() return true end, 2) And also this issue seems to be somewhat popular, judging by the number of positive upvotes here. |
No problem at all, I appreciate everyone's viewpoint on the matter. I continue to think about this, and at times I sometimes think about implementing it, and other times not 😆. I think what I might do is add an option for this in another branch, and see the impact it has in real world use cases. But that is no guarantee that it will be added or not, just a test. I'll look into it over the weekend |
Apologies for the delay, I got busy last weekend. It can be installed using
then setting collapse mode to "Functions" ( Please let me know if there are any issues with it, but as I mentioned earlier it is not guaranteed to be added. |
Made a comment in PR. Definitely interested also in one-ilne |
Added in support for one line ifs as well following this syntax:
|
In particular, this is much nicer for keymaps. See JohnnyMorganz/StyLua#248
Understanding this is closed, I'm still left thinking either I am missing where a clear explanation is found or it warrants being explained better. The fact that: local _ = { function() somefunc() end } being by default expanded to: local _ = {
function()
somefunc()
end,
} is completely counterintuitive when the only mitigating setting is And that actually seems maybe even more terrible, but I've spent a bit of time trying to understand why I have to set a collapse setting to prevent an expansion. It is a strange situation. I hope I'm being clear. |
So, there are a couple of reasons for this. Firstly, the collapse option was not originally intended but was added due to popular demand (meaning no other options were really considered at the time). But mainly, one of the original goals for StyLua (similar to prettier or black) is to provide automated and consistent formatting. This means we do not rely on the input code style to determine the output code style, otherwise different starting points would produce different results. (The only place this is violated is for deciding whether to collapse tables if they were already expanded - prettier has a good discussion about it at https://prettier.io/docs/en/rationale#multi-line-objects, and also discusses the concept for "formatter reversibility") A key reason to enforce consistency is the original principle to reduce bikeshedding. If we rely on input style, then we lose out on this: now, style nit comments become a thing again (e.g., a review comment that says "nit: please collapse this so that stylua leaves it collapsed") The idea that you suggest would unfortunately break this goal, and I try to shy away from reliance on input as much as possible |
I think it would be useful to allow definitions of "simple" functions to be placed on single line. So that, for example, the code
func(function() return true end, 2)
will not be formatted asThe notion of "simple function" might be aligned with "simple
if
" from #232. This is also somewhat related to #188.The text was updated successfully, but these errors were encountered: