Make identifier lambdas parenless #1692
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: Test Plan: Reviewers: CC:
Summary: Stop printing unneeded parens around single identifier lambdas. This brings us closer to compatibility with the JS convention. Test Plan: Reviewers: CC:
jordwalke
commented
Dec 9, 2017
| @@ -19,42 +19,42 @@ fff >>= xx(yy) >>= aa(bb); | |||
| fff >>= (xx(yy) >>= aa(bb)); | |||
|
|
|||
There was a problem hiding this comment.
@glennsl This is the kind of stuff this diff cleans up.
jordwalke
commented
Dec 9, 2017
| @@ -205,7 +205,7 @@ let selfClosing3 = | |||
| b="cause the entire thing to wrap" | |||
| />; | |||
|
|
|||
There was a problem hiding this comment.
And jsx callback arguments.
| @@ -1084,7 +1084,7 @@ let server = { | |||
| ) | |||
| ) | |||
| >>= ( | |||
| (body) => | |||
| body => | |||
There was a problem hiding this comment.
Hmm forgot this case in the callback PR.
The body => should dock on the same line as >>= (, shouldn't it?
There was a problem hiding this comment.
I'm not sure - printing these have bigger issues - like knowing when exactly we can omit the parens. We're fairly certain that determining when the () can be omitted is such a hard problem that it requires integration with the parser.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This partially addresses the issue that @bobzhang and @glennsl brought up about the new syntax having too many parens. I tried formatting a large project and noticed that the most jarring use of parens was:
We can't really remove the parens around record arguments to lambdas at the moment because then it opens up this precedent which we cannot uphold for tuples.
But still, single identifier lambda arguments were the biggest offender of paren-fatigue in my example project.
cc @IwanKaramazow @chenglou @rickyvetter