goldstein.w.n added a comment. In D137181#3914292 <https://reviews.llvm.org/D137181#3914292>, @owenpan wrote:
> In D137181#3914100 <https://reviews.llvm.org/D137181#3914100>, @goldstein.w.n > wrote: > >> Err I'm not sure that's right. I thought we where going for the C-code >> should start at the column that the 'd' in define is. >> So it would be: >> >> #define X \ >> switch (x) { \ >> case 0: \ >> break; \ >> default: \ >> break; \ >> } > > That doesn't make sense IMO. Here is an example: > > $ cat foo.cpp > #if X > #define FOO \ > if (a) \ > b; > #endif > $ clang-format -style="{BasedOnStyle: Chromium, IndentWidth: 4}" foo.cpp > > out1 > $ diff foo.cpp out1 > $ clang-format -style="{BasedOnStyle: Chromium, IndentWidth: 4, > PPIndentWidth: 1}" foo.cpp > out2 > > Now what should out2 look like? Well, it should be no different than out1 > (and foo.cpp) because `IndentPPDirectives` is still `None` by default. > However, the current patch outputs the following instead: > > #if X > #define FOO \ > if (a) \ > b; > #endif > > So I really think the first line of a macro definition body should be > indented `IndentWidth - 1` relative to the letter `d` in `define` no matter > what `IndentPPDirectives` and `PPIndentWidth` are set to. Fair enough. Adding something like: if (PPIndentWidth != Style.IndentWidth) Indent += Style.IndentWidth - 1; Works for that and all existing tests pass + added a few tests with varied indentwidth / ppindentwidth. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137181/new/ https://reviews.llvm.org/D137181 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
