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

Fix crash when breaking up long comment after simple lambda body #203

Merged
merged 3 commits into from
Feb 19, 2020

Conversation

dansanduleac
Copy link
Contributor

@dansanduleac dansanduleac commented Feb 17, 2020

Before this PR

A long comment that follows an otherwise simple lambda body (no inner levels) can cause a crash if it ends up being chunked onto multiple lines by the CommentsHelper.

arg -> true // long comment might end up
            // being chunked oops

After this PR

==COMMIT_MSG==
Make lambda/assignment logic more resilient so it doesn't crash when encountering long comments.
==COMMIT_MSG==

Possible downsides?

This currently means that if the lambda / assignment body is not complex but has a long comment which doesn't fit on one line, the body will always end up on the 2nd line:

arg ->
        true // long comment might end up
             // being chunked oops

If this is not desirable, we could revert the heuristic to what it was before, but be resilient in the next method down the line tryInlinePrefixOntoCurrentLine so we don't crash if no inner level is found.

@changelog-app
Copy link

changelog-app bot commented Feb 17, 2020

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Make lambda/assignment logic more resilient so it doesn't crash when encountering long comments.

Check the box to generate changelog(s)

  • Generate changelog entry

@policy-bot policy-bot bot requested a review from ferozco February 17, 2020 11:44
@ferozco
Copy link
Contributor

ferozco commented Feb 18, 2020

Instead of breaking the line because of trailing comment could we move the comment to be on a separate line above the simple lambda?

@dansanduleac
Copy link
Contributor Author

We possibly could, but that's a pretty big deviation from how the formatter was set up to deal with comments, so it might break other things

@ferozco
Copy link
Contributor

ferozco commented Feb 19, 2020

ok, if you don't think its worth it 👍

@bulldozer-bot bulldozer-bot bot merged commit 58fb486 into develop Feb 19, 2020
@bulldozer-bot bulldozer-bot bot deleted the ds/fix-crash branch February 19, 2020 00:46
@svc-autorelease
Copy link
Collaborator

Released 0.3.26

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

Successfully merging this pull request may close these issues.

3 participants