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

Whitespace before/after curly braces is overly prescriptive #80

Closed
thauk-copperleaf opened this issue Sep 11, 2017 · 2 comments
Closed

Comments

@thauk-copperleaf
Copy link

The rule for whitespace before/after curly braces is most appropriate when curly braces are used for blocks. But this rule is also enforced in other situations, like passing inlined lambda parameters to functions (e.g. associateBy). That means it creates an implicit rule about spacing with function parameters where no rule exists currently.

So something like this would be considered acceptable to ktlint but has obviously inconsistent styling:

val foo = listOf("Foo")
val bar = foo.associateBy( { it.length } , { it } )
val baz = someFunc(1, 42)

(I personally think foo.associateBy( { it.length } , { it } ) is less readable than foo.associateBy({it.length} , {it}))

I propose that this whitespace rule be changed to only apply to block level.

@shyiko
Copy link
Collaborator

shyiko commented Sep 11, 2017

@thauk-copperleaf

ktlint tries to adhere to the default IDEA's settings whenever possible (to stay compatible with the Ctrl+Alt+L) and so the expected code style in this case is:

val foo = listOf("Foo")
val bar = foo.associateBy({ it.length }, { it })
val baz = someFunc(1, 42)

foo.associateBy( { it.length } , { it } ) triggers lint errors after } and so it's not going to pass validation (verified locally using ktlint@0.9.2). I would expect a lint error between ( and { but it seems like this case isn't covered (I'll take care of it over the weekend).

I'll keep this ticket open until a new release (with a fix for ( {) is published but other than that it looks like everything is in order.

shyiko added a commit that referenced this issue Oct 11, 2017
…as correct, while only "({" should be) (curly-spacing) (#80)
@shyiko
Copy link
Collaborator

shyiko commented Oct 11, 2017

"( {" formatting fixed in 0.10.0.

@shyiko shyiko closed this as completed Oct 11, 2017
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

No branches or pull requests

2 participants