[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments
Owen Pan via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 6 14:08:30 PST 2022
owenpan added a comment.
Can you rebase (preferably after landing D138402 <https://reviews.llvm.org/D138402>)?
================
Comment at: clang/lib/Format/WhitespaceManager.cpp:991
+ bool DontAlignThisComment =
+ i > 0 && Changes[i - 1].Tok->is(TT_NamespaceRBrace);
bool WasAlignedWithStartOfNextLine = false;
----------------
Don't we still need to check `Changes[i].NewlinesBefore == 0`? How would this format the code below with `FixNamespaceComments: false`?
```
namespace A {
...
}
// comment
```
================
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));
+ }
+
----------------
Was this already done in D138402?
================
Comment at: clang/unittests/Format/FormatTestComments.cpp:3094
+TEST_F(FormatTestComments, DontAlignNamespaceComments) {
+ FormatStyle Style = getLLVMStyle();
+ Style.NamespaceIndentation = FormatStyle::NI_All;
----------------
Should we add tests for `FixNamespaceComments: false`?
================
Comment at: clang/unittests/Format/FormatTestComments.cpp:3137
+ " } // namespace C\n"
+ " }// TESTSUITE(B)\n"
+ "} // NaMeSpAcE A",
----------------
Why would `TCAS_Leave` result in no space before the trailing comment?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138263/new/
https://reviews.llvm.org/D138263
More information about the cfe-commits
mailing list