[PATCH] D95168: [clang-format] Add InsertBraces option
MyDeveloperDay via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 5 03:35:55 PST 2021
MyDeveloperDay added inline comments.
================
Comment at: clang/unittests/Format/FormatTest.cpp:17996
+ format(ForSourceLong, Style));
+}
+
----------------
tiagoma wrote:
> 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.
The problem with this statement is as soon as you commit, we'd get that defect raised, that is ok its just are you going to stay around long enough to finish it completely? ;-) if not then this puts burden on those who hang out here all the time.
Ideally its probably worth following through with a complete implementation before landing (or as best you can), there is no rush right?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95168/new/
https://reviews.llvm.org/D95168
More information about the cfe-commits
mailing list