[PATCH] D109557: Adds a BlockIndent option to AlignAfterOpenBracket

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D109557#3066854 <https://reviews.llvm.org/D109557#3066854>, @csmulhern wrote:

> In D109557#3063681 <https://reviews.llvm.org/D109557#3063681>, @MyDeveloperDay wrote:
>
>> Personally I'm not convinced there wouldn't be people who want this for function declarations and definitions
>>
>>   Function(
>>       param1,
>>       param2,
>>       param3
>>   );
>>
>> but don't want this
>>
>>   if (
>>      foo()
>>   )
>>   ....
>
> To be clear, the if styling above would only occur if `foo()` was long enough to line wrap. Not all instances of `if`. However, I agree that could be true, and the existing clang-format code clearly treats indentation inside if / for / while differently than e.g. function calls. The existing AlignAfterOpenBracket options for example do not apply to the former for instance, which is why we see the weird indentation in the current revision where the opening brace is not followed by a line break, but the closing brace is preceded by one.

So we would still have the chance for a breaking if condition? There should be a test to document that! But I'm also not sure this should happen, but lets see, nobody is forced to chose `BlockIndent`.

Otherwise this is fine for me.



================
Comment at: clang/include/clang/Format/Format.h:100
+    ///
+    /// Note: This currently only applies to parentheses.
+    BAS_BlockIndent,
----------------
Maybe highlight this with the warning block?
Or correct it. ;)


================
Comment at: clang/lib/Format/ContinuationIndenter.cpp:951
 
+  if (PreviousNonComment && PreviousNonComment->is(tok::l_paren)) {
+    State.Stack.back().BreakBeforeClosingParen =
----------------
Remove the braces


================
Comment at: clang/unittests/Format/FormatTest.cpp:22453
+  Style.AllowAllArgumentsOnNextLine = false;
+  Style.AllowAllConstructorInitializersOnNextLine = false;
+  Style.AllowAllParametersOfDeclarationOnNextLine = false;
----------------
How does it behave without these?


================
Comment at: clang/unittests/Format/FormatTest.cpp:22560
+
+  verifyFormat("if (quitelongarg !=\n"
+               "    (alsolongarg - 1)) { // ABC is a very longgggggggggggg "
----------------
I think you should add a test without the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109557