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

Add option to add spaces in function definitions #832

Closed
alerque opened this issue Dec 12, 2023 · 5 comments · Fixed by #839
Closed

Add option to add spaces in function definitions #832

alerque opened this issue Dec 12, 2023 · 5 comments · Fixed by #839
Labels
enhancement New feature or request requires option This feature request would require option configuration

Comments

@alerque
Copy link
Contributor

alerque commented Dec 12, 2023

One style choice I've made extensively across many projects that clashes with stylua is to add spaces in function definitions between the function name and arguments.

-- like this
local function foo (arg) end
foo(6)

-- or like this
local foo = function (arg) end
foo(6)

-- not this
local function(arg) end
foo(6)

-- and not this
local foo = function(arg) end
foo(6)

This style gives some visual differentiation between function definitions and function calls. In these simple examples it is relatively obvious, but it is possible for closures to get a bit confusing. Obviously this is an opinionated choice, but I'm not the only one to make this choice. I got it from other projects and just thought it was valuable enough to continue.

I can understand if people don't want this to be the default (as I would set it) but at least an option would be nice.

@JohnnyMorganz JohnnyMorganz added enhancement New feature or request requires option This feature request would require option configuration labels Dec 23, 2023
@alerque
Copy link
Contributor Author

alerque commented Dec 23, 2023

I'm encouraged by the triage labeling that this wasn't just rejected out of hand. Encouraged enough I'd consider looking into contributing the feature myself if I can make heads or tails of the code.

The only contra-indicator to investing some time in this is #628, a feature I don't personally need since I like the current default but it has been open for a long time. Are contributions that add options like this going to have a way forward?

@JohnnyMorganz
Copy link
Owner

JohnnyMorganz commented Dec 23, 2023

I will be thinking about it over the next few days and get back to you :)

I am leaning towards accepting #620, but not completed convinced yet. I would wait until a final decision was made on #620 before moving forward with this, so that you don't waste your time. (I think your comment on #620 sums up my same thinking pretty well, I just need to personally get over the hurdle that there is no turning back once we start accepting options).

Started adding the "requires option" label today so that I can see out of all our current feature requests what would be interested in allowing options.

@alerque
Copy link
Contributor Author

alerque commented May 29, 2024

So ... how was your Christmas? Santa didn't leave the cookies in a "620" shape?

@alerque
Copy link
Contributor Author

alerque commented May 29, 2024

I've got impatient enough to cleanup some of my projects that I implemented this for myself. My PR #839 has been upgraded from just a new config option to actually implementing the formatting and tests.

@JohnnyMorganz
Copy link
Owner

Apologies for the radio silence, I haven't had the chance to do much stylua work for a while.

Cutting to the chase, yeah let's go for it :) I'm happy to start adding more options in.

I want to release a "2.0" major version bump as the next release containing this expanded option setup (mainly to signal the shift in direction). This however is currently blocked on a new parser update with #854 that currently has some stack overflow issues. So that means it might take a little more time for this option to make its way into an actual release. That said, I'll try to review the PR soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request requires option This feature request would require option configuration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants