[PATCH] D156259: Fix a bug that erroneously placed function arguments on a new line despite all arguments being able to fit on the same line.

[Jon Phillips via Phabricator via cfe-commits]

jp4a50 marked 3 inline comments as done.
jp4a50 added a comment.

All comments addressed. PTAL and let me know if there are any other blockers for merge.



================
Comment at: clang/lib/Format/ContinuationIndenter.cpp:664-666
+    if ((Style.Language == FormatStyle::LK_Cpp ||
+         Style.Language == FormatStyle::LK_ObjC) &&
+        !Current.is(tok::comment) && PrevNonComment &&
----------------
HazardyKnusperkeks wrote:
> jp4a50 wrote:
> > HazardyKnusperkeks wrote:
> > > jp4a50 wrote:
> > > > HazardyKnusperkeks wrote:
> > > > > Could move this out of the lambda.
> > > > Not sure exactly what you're referring to here. Initialization of `PrevNonComment`?
> > > > Not sure exactly what you're referring to here. Initialization of `PrevNonComment`?
> > > 
> > > No everything in the if before you go into the `PrevNonComment`. It is highlighted if you hover the comment.
> > Ah, I see, thanks. Technically I could move that outside the lambda. I'm just not sure how it's better? Are you making the suggestion because it would make it clearer that this logic only applies to cpp/objc and/or because it means we could avoid capturing `Style`?
> Can do how you want, I think avoid capturing `Style` would be nice. And you wouldn't search for `PrevNonComment` if you don't need to. This could also be done be reordering the statements and a new `return`.
I kept all conditions inside the lambda in the end since moving any out would have made the `DisallowLineBreaksOnThisLine` not reflect all conditions and I wanted to keep that.

Instead I've changed all conditions to the style of early return for consistency and ordered them in the way that I think makes them most readable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156259/new/

https://reviews.llvm.org/D156259