anastasiia_lukianenko added inline comments.
================ Comment at: clang/lib/Format/Format.cpp:893 /*BeforeLambdaBody=*/false, + /*BeforeStructInitialization=*/false, /*BeforeWhile=*/false, ---------------- MyDeveloperDay wrote: > 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 > > Ok, thank you. ================ Comment at: clang/unittests/Format/FormatTest.cpp:5063 + " a = 1,\n" + " b = 2,\n" + "};", ---------------- MyDeveloperDay wrote: > 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? I agree with you. And I'll add these tests to patch. 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