anastasiia_lukianenko added inline comments.

================
Comment at: clang/unittests/Format/FormatTest.cpp:5075
+               "};",
+               Style);
+}
----------------
MyDeveloperDay wrote:
> I wonder if there should be other options
> 
> i.e.
> 
> ```
> struct new_struct struct_name = {
>     a = 1
> };
> ```
> ```
> struct new_struct struct_name = { a = 1 };
> ```
> 
> ```
> struct new_struct struct_name = 
> { 
>     a = 1,
> };
> ```
> 
> Again I don't want to discourage you, but this is very much related to one of 
> the key problem areas of clang-format that it really doesn't do well, so its 
> one of the places users ALWAY have to add //clang-format off
> 
> e.g.
> 
> ```
> Status GradForBinaryCwise(FunctionDef* g, std::vector<FDH::Node> body) {
>   // clang-format off
>   std::vector<FDH::Node> nodes = {
>     {{"sx"}, "Shape", {"x"}},
>     {{"sy"}, "Shape", {"y"}},
>   };
>   // clang-format on
> ```
> 
> If we are going to fix this to some extent we should fix it for all those 
> cases too. I understand that that might be beyond what you are trying to 
> achieve, but I think its important that we explore what might be needed to 
> satisfy being able to get rid of all these //clang-format off cases as some 
> point and you've woken the beast in this area ;-)
> 
> To be honest fixing this is one of biggest things (and very commendable if 
> you want to focus on it) and whilst your fix is along the right lines, it 
> only addresses the struct cases (and not the class case example (as above) 
> and it perhaps doesn't cover where there might be something before the struct 
> as in 
> 
> ```
> static struct X Y = {
> }
> ```
> 
> If your interested in pursuing this still, I'm happy to try and help you walk 
> through it, where I can. However you'll have to bare with me as it might be 
> beyond my skill set.
> 
> 
I understand what you mean, but at the moment options such as "AfterStruct" are 
also implemented with checking the first word in a line (not considering cases 
with "static" word). It seems to me that the option that I add should only 
concern structures, and new configurations  should be created for the class, 
etc. (BreakBeforeClassInit, BreakBeforeEnumInit), so that the formatting would 
be more flexible.


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
  • [PATCH] D91949: [cla... Anastasiia Lukianenko via Phabricator via cfe-commits
    • [PATCH] D91949:... Anastasiia Lukianenko via Phabricator via cfe-commits
    • [PATCH] D91949:... MyDeveloperDay via Phabricator via cfe-commits

Reply via email to