[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

Björn Schäpers via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Apr 16 12:01:52 PDT 2023


HazardyKnusperkeks added a comment.

In D148467#4272173 <https://reviews.llvm.org/D148467#4272173>, @MyDeveloperDay wrote:

> I'm not convinced `AfterCSharpProperty`  is a good name, open for suggestions

I've not enough domain knowledge to name it.



================
Comment at: clang/include/clang/Format/Format.h:1220
+    /// \endcode
+    bool AfterCSharpProperty;
   };
----------------
Please sort. :)


================
Comment at: clang/include/clang/Format/Format.h:4269
 
   bool operator==(const FormatStyle &R) const {
     return AccessModifierOffset == R.AccessModifierOffset &&
----------------
It's missing in operator==. And I think we should add operator== to `BraceWrappingFlags` and use that here.


================
Comment at: clang/lib/Format/Format.cpp:184
     IO.mapOptional("AfterFunction", Wrapping.AfterFunction);
+    IO.mapOptional("AfterCSharpProperty", Wrapping.AfterCSharpProperty);
     IO.mapOptional("AfterNamespace", Wrapping.AfterNamespace);
----------------
A bit up.


================
Comment at: clang/unittests/Format/FormatTestCSharp.cpp:1633
+               "{\n"
+               "    string Foo { set; get }\n"
+               "}\n",
----------------
MyDeveloperDay wrote:
> Hmm.. maybe this should be 
> 
> ```
> string Foo 
> {
>     set;get
> }
> ```
I'm no C# guy, but when we split the lines, I think get and set should be on it's own line.


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

https://reviews.llvm.org/D148467



More information about the cfe-commits mailing list