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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to