[PATCH] D95168: [clang-format] Add InsertBraces option

[Tiago Macarios via Phabricator via cfe-commits]

tiagoma marked 5 inline comments as done.
tiagoma added inline comments.


================
Comment at: clang/unittests/Format/FormatTest.cpp:17996
+            format(ForSourceLong, Style));
+}
+
----------------
njames93 wrote:
> MyDeveloperDay wrote:
> > MyDeveloperDay wrote:
> > > MyDeveloperDay wrote:
> > > > are you testing do/while? 
> > > whilst people discuss the ethics of modifying the code ;-) 
> > > 
> > > Can you add some comment based examples
> > > 
> > > ```
> > > if (condition) // my test
> > >       you_do_you();
> > > 
> > > if (condition)
> > >       you_do_you(); // my test
> > > ```
> > bonus points..
> > 
> > ```
> > if /*condition*/ (condition) /*condition*/
> > /*condition*/      you_do_you(); /*condition*/
> > ```
> Should also add test for chained conditionals just to make sure the semantics of the code doesn't change.
> ```lang=c
> if (A)
>   if (B)
>     callAB();
>   else
>     callA();
> else if (B)
>   callB();
> else
>   call();```
do/while are not supported in it's current form. We would need to change the logic to add more state. I can have a look at it after this patch is accepted.


================
Comment at: clang/unittests/Format/FormatTest.cpp:17996
+            format(ForSourceLong, Style));
+}
+
----------------
tiagoma wrote:
> njames93 wrote:
> > MyDeveloperDay wrote:
> > > MyDeveloperDay wrote:
> > > > MyDeveloperDay wrote:
> > > > > are you testing do/while? 
> > > > whilst people discuss the ethics of modifying the code ;-) 
> > > > 
> > > > Can you add some comment based examples
> > > > 
> > > > ```
> > > > if (condition) // my test
> > > >       you_do_you();
> > > > 
> > > > if (condition)
> > > >       you_do_you(); // my test
> > > > ```
> > > bonus points..
> > > 
> > > ```
> > > if /*condition*/ (condition) /*condition*/
> > > /*condition*/      you_do_you(); /*condition*/
> > > ```
> > Should also add test for chained conditionals just to make sure the semantics of the code doesn't change.
> > ```lang=c
> > if (A)
> >   if (B)
> >     callAB();
> >   else
> >     callA();
> > else if (B)
> >   callB();
> > else
> >   call();```
> do/while are not supported in it's current form. We would need to change the logic to add more state. I can have a look at it after this patch is accepted.
This specific test will fail by itself, ie:

```
StringRef Test = "if /*condition*/ (condition)  /*condition*/\n"
                 "  /*condition*/ you_do_you(); /*condition*/";

verifyFormat(Test);
```

AFAICT it is because test::messUp causes the 2nd and 3rd comment to stay on the same line. I added a variation of the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95168