[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

[Noah Goldstein via Phabricator via cfe-commits]

goldstein.w.n added inline comments.


================
Comment at: clang/unittests/Format/FormatTest.cpp:5024
                style);
+
   verifyFormat("#if 1\n"
----------------
owenpan wrote:
> Don't add an empty line here.
Will drop for V2.


================
Comment at: clang/unittests/Format/FormatTest.cpp:5045
+
+  style.ColumnLimit = 40;
+  style.IndentPPDirectives = FormatStyle::PPDIS_None;
----------------
owenpan wrote:
> Do you need it?
No, copied along when I was looking for examples of unit-tests with trailing '\'. I'll drop for V2


================
Comment at: clang/unittests/Format/FormatTest.cpp:5047
+  style.IndentPPDirectives = FormatStyle::PPDIS_None;
+  verifyFormat("#ifdef foo\n"
+               "#define bar() \\\n"
----------------
owenpan wrote:
> Do you need to enclose the macro definition in `#ifdef`/`#endif`?
No, but figured since this change is related to PP indentation the tests should have it. You would prefer I make the test just:

```
  verifyFormat("#define bar() \\\n"
               "    if (A) {  \\\n"
               "        B();  \\\n"
               "    }         \\\n"
               "    C();\n",
               "#define bar() if (A) { B(); } C();\n",
               style);
```

?


================
Comment at: clang/unittests/Format/FormatTest.cpp:5054-5056
+               "#ifdef foo\n"
+               "#define bar() if (A) { B(); } C();\n"
+               "#endif",
----------------
owenpan wrote:
> You can delete them.
Them being the ifdef/endif? If so will do for V2.


================
Comment at: clang/unittests/Format/FormatTest.cpp:5066-5068
+               "#ifdef foo\n"
+               "#define bar() if (A) { B(); } C();\n"
+               "#endif",
----------------
owenpan wrote:
> Delete them or change to multiline format like lines 5048-5052.
Delete the test of the ifndef/endif? Can do either for V2.


================
Comment at: clang/unittests/Format/FormatTest.cpp:5078-5080
+               "#ifdef foo\n"
+               "#define bar() if (A) { B(); } C();\n"
+               "#endif",
----------------
owenpan wrote:
> Same here.
Same question as above, otherwise will do for V2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181