MyDeveloperDay added inline comments.

================
Comment at: clang/lib/Format/Format.cpp:893
                              /*BeforeLambdaBody=*/false,
+                             /*BeforeStructInitialization=*/false,
                              /*BeforeWhile=*/false,
----------------
anastasiia_lukianenko wrote:
> MyDeveloperDay wrote:
> > I believe there are 3 places where BraceWrapping is initialized you seem to 
> > only be doing 2 of them
> Sorry, I didn't find one more place. Can you please say where it is?
`expandPresets()` - twice
`getLLVMStyle()` - once




================
Comment at: clang/unittests/Format/FormatTest.cpp:5063
+               "    a = 1,\n"
+               "    b = 2,\n"
+               "};",
----------------
anastasiia_lukianenko wrote:
> MyDeveloperDay wrote:
> > could you add an additional test without the trailing `,`
> Yes, I can add the test, but before I want to discuss the expected result.
> Now with my patch and BeforeStructInitialization = false the behavior is the 
> following:
> 
> ```
> struct new_struct struct_name = {a = 1};
> struct new_struct struct_name = {a = 1, b = 2};
> ```
> And with BeforeStructInitialization = true:
> 
> ```
> struct new_struct struct_name =
> {a = 1};
> struct new_struct struct_name =
> {a = 1, b = 2};
> ```
> Is it okay?
that is kind of what I expected, and adding the `,` is a known trick to cause 
it to expand the struct out.

I think its worth putting a test in, but I'll ask the same question what do you 
think it should look like?


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

https://reviews.llvm.org/D91949

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

Reply via email to