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

feat: Add more granular control over call parentheses #330

Merged
merged 2 commits into from Jan 10, 2022
Merged

feat: Add more granular control over call parentheses #330

merged 2 commits into from Jan 10, 2022

Conversation

shadmansaleh
Copy link
Contributor

Adds new option call_parentheses with possible values Always, NoTable,
None (Default: Always)

When call_parentheses is set to Always stylua applies call parentheses
all the time.

xyz({ a = 1, b = 2 })
abc('Hello')

When set to NoTable it omits parentheses on function call with
single table argument.

xyz { a = 1, b = 2 }
abc('Hello')

And when it's None stylua omits parentheses on function call with
single table or string argument. This is same as no_call_parentheses = true

xyz { a = 1, b = 2 }
abc 'Hello'

closes #329

@clason
Copy link

clason commented Jan 3, 2022

For the sake of symmetry, it'd be nice to allow for the converse option (NoString), too.

And personally I'd flip it to have TableOnly and StringOnly (opt-in instead of opt-out) ;)

CHANGELOG.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
tests/test_call_parentheses.rs Outdated Show resolved Hide resolved
src/formatters/functions.rs Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@shadmansaleh
Copy link
Contributor Author

@JohnnyMorganz let me know what ypu think about clasons suggestions

@JohnnyMorganz
Copy link
Owner

Ok, I think adding an option for strings is sensible as well for the sake of consistency, it isn't really adding anything major either

Technically these are opt-outs from the default (Always), so I think NoString/NoTable are more appropriate, although StringOnly/TableOnly kinda do sound nicer. Not sure

Adds new option call_parentheses with possible values Always, NoString, NoTable,
     None (Default: Always)

When call_parentheses is set to Always stylua applies call parentheses
all the time.
```lua
xyz({ a = 1, b = 2 })
abc('Hello')
When set to NoString it omits parentheses on function call with
single table argument.
```lua
xyz({a = 1, b = 2 })
abc 'Hello'
```
```
Similarly when set to NoTable it omits parentheses on function call with
single table argument.
```lua
xyz { a = 1, b = 2 }
abc('Hello')
```
And when it's None stylua omits parentheses on function call with
single table or string argument. This is same as `no_call_parentheses =
true`
```lua
xyz { a = 1, b = 2 }
abc 'Hello'
```
@shadmansaleh
Copy link
Contributor Author

I've added NoString

Copy link
Owner

@JohnnyMorganz JohnnyMorganz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good to me, the only thing I'm not sure of is whether to go with NoString/NoTable or TableOnly/StringOnly

@shadmansaleh
Copy link
Contributor Author

shadmansaleh commented Jan 8, 2022

Let me add one more to that confusion . How about NoSingleString, NoSingleTable . I think that conveys the correct message about what the option does . NoString/TableOnly sounds like parens will be dropped strimg args always .

@JohnnyMorganz
Copy link
Owner

JohnnyMorganz commented Jan 9, 2022

I agree, that sounds better. Lets go with NoSingleString / NoSingleTable - I think the verbosity of the name is reasonable
(There is also NotAroundSingleString / NotAroundSingleTable, but maybe that's too verbose)

@JohnnyMorganz
Copy link
Owner

Thank you!

@JohnnyMorganz JohnnyMorganz merged commit 6272fdf into JohnnyMorganz:master Jan 10, 2022
@shadmansaleh shadmansaleh deleted the feat/call_parens branch January 10, 2022 11:23
@clason
Copy link

clason commented Jan 28, 2022

@JohnnyMorganz what do you think about a new release? With this change, we could consider enforcing StyLua on Neovim's codebase :)

@JohnnyMorganz
Copy link
Owner

@JohnnyMorganz what do you think about a new release? With this change, we could consider enforcing StyLua on Neovim's codebase :)

I'm just waiting on a new full-moon release upstream, hopefully by the end of the week. Once that's out, I'll cut a new stylua release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Add an option to remove parethesis arround table call
3 participants