[PATCH] D94661: [clang-format] [PR19056] Add support for access modifiers indentation

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.


================
Comment at: clang/unittests/Format/FormatTest.cpp:17874
+  verifyFormat("class C {\n"
+               "    int i;\n"
+               "};\n",
----------------
So this is indented 4 because the Access modifier is 4? when the IndentWidth is 2? 

but public below is indented 2.. so now I'm really confused?


================
Comment at: clang/unittests/Format/FormatTest.cpp:17882-17908
+  {
+    const char *Expected = "struct S {\n"
+                           "  private:\n"
+                           "    class C {\n"
+                           "        int j;\n"
+                           "\n"
+                           "      public:\n"
----------------
Budovi wrote:
> When you re-create this test using `verifyFormat`, you get a weird results, i.e. if you keep the newlines before access modifiers, the test fails because it thinks they should not be there, but if you remove them, test fails again, because the formatter includes them instead.
> 
> It's a good question whether it is a side-effect of `test::messUp` or a different bug, I'm not sure.
As a drive by comment, its my experience when verifyFormat fails its often a sign that there is a bug which means clang-format cannot always format from arbitrary text, which really it should.

Ultimately this bug will get logged by someone who doesn't understand your change as well as you do. That fix will also subtly end up creating a different bug in another corner case you you haven't covered in your original tests

We spiral down from there...my 2c worth.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94661