HazardyKnusperkeks added inline comments.
================ Comment at: clang/lib/Format/WhitespaceManager.cpp:991 + bool DontAlignThisComment = + i > 0 && Changes[i - 1].Tok->is(TT_NamespaceRBrace); bool WasAlignedWithStartOfNextLine = false; ---------------- owenpan wrote: > Don't we still need to check `Changes[i].NewlinesBefore == 0`? How would this > format the code below with `FixNamespaceComments: false`? > ``` > namespace A { > ... > } > // comment > ``` Will check. ================ Comment at: clang/unittests/Format/FormatTestComments.cpp:71-83 + void verifyFormat(llvm::StringRef Expected, llvm::StringRef Code, + const FormatStyle &Style = getLLVMStyle()) { + EXPECT_EQ(Expected.str(), format(Expected, Style)) + << "Expected code is not stable"; + EXPECT_EQ(Expected.str(), format(test::messUp(Code), Style)); + } + ---------------- owenpan wrote: > Was this already done in D138402? The first yes, the second no. I did just not update this diff, my commit is updated. ================ Comment at: clang/unittests/Format/FormatTestComments.cpp:3094 +TEST_F(FormatTestComments, DontAlignNamespaceComments) { + FormatStyle Style = getLLVMStyle(); + Style.NamespaceIndentation = FormatStyle::NI_All; ---------------- owenpan wrote: > Should we add tests for `FixNamespaceComments: false`? Will do. ================ Comment at: clang/unittests/Format/FormatTestComments.cpp:3137 + " } // namespace C\n" + " }// TESTSUITE(B)\n" + "} // NaMeSpAcE A", ---------------- owenpan wrote: > Why would `TCAS_Leave` result in no space before the trailing comment? It just did, didn't investigate or decide. Most likely clang-format just adds it there and the space just comes from the other formatting, which is disabled with `Leave`. I'd say this is fine, `Leave` just means the coder decides on the position of the comments, and if that comment is added he can just move it around and clang-format will not touch it any further. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138263/new/ https://reviews.llvm.org/D138263 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits