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

Inserting a block-comment before a property call chain wraps each chained element #1628

Closed
srawlins opened this issue Dec 17, 2024 · 2 comments · Fixed by #1629
Closed

Inserting a block-comment before a property call chain wraps each chained element #1628

srawlins opened this issue Dec 17, 2024 · 2 comments · Fixed by #1629

Comments

@srawlins
Copy link
Member

I was surprised by this in the new formatting. Here is an input text:

bool f(bool b, int i) {
  return // Check b.
  b == true || i.isEven;
}

The formatter likes all that code on one line. If I want to add a comment for the i.isEven expression, then it insists on splitting:

bool f(bool b, int i) {
  return // Check b.
  b == true ||
          // Check i.
          i
          .isEven;
}

🤕 Ouch. If this is intentional, I'll trust your research, and you can close.

@munificent
Copy link
Member

Yeah, the new formatter sometimes doesn't know what to do with comments in unusual places. It tends to attach them to the subsequent token, which is always owned by the deepest subexpression. That in turn means that subexpression contains a newline (from the line comment) which forces all of the surrounding expressions to split.

I kept running into these bugs and finally in #1615 found a more systematic way to handle them. It looks like this issue is fixed on the main branch already, so it seems #1615 is doing its job. I'll add a regression test for this.

munificent added a commit that referenced this issue Dec 17, 2024
Looks like this was already fixed by #1615.

Close #1628.
@srawlins
Copy link
Member Author

Ah I didn't check closed bugs. Thanks Bob!

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 a pull request may close this issue.

2 participants