[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2976-2977
 
+    FormatToken *LBrace = FormatTok;
+    LBrace->setFinalizedType(TT_NamespaceLBrace);
+
----------------
To be future-proof, I'd set the type at the beginning of the block, right after line 2957.


================
Comment at: clang/lib/Format/WhitespaceManager.cpp:991-993
+    bool DontAlignThisComment = i > 0 &&
+                                Changes[i - 1].Tok->is(TT_NamespaceRBrace) &&
+                                Changes[i].NewlinesBefore == 0;
----------------
To minimize the diff.


================
Comment at: clang/unittests/Format/FormatTestComments.cpp:3086
+
+  llvm::StringRef Input = "namespace A {\n"
+                          "  TESTSUITE(B) {\n"
----------------



================
Comment at: clang/unittests/Format/FormatTestComments.cpp:3182
+               "} // namespace A\n"
+               "  // Comment", Input, Style);
+
----------------
And put the test case in a `#if 0` block? I don't think it should be aligned with the `namespace` comment.


================
Comment at: clang/unittests/Format/FormatTestComments.cpp:3185-3190
+  verifyFormat("namespace A {\n"
+               "  int Foo;\n"
+               "  int Bar;\n"
+               "}\n"
+               "// Comment",
+               Input, Style);
----------------



================
Comment at: clang/unittests/Format/FormatTestComments.cpp:3137
+                            "    }          // namespace C\n"
+                            "  }// TESTSUITE(B)\n"
+                            "} // NaMeSpAcE A",
----------------
HazardyKnusperkeks wrote:
> 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.
IMO there should be a single space or tab between the closing `namespace` brace and the `namespace` comment (existing or inserted) as `AlignTrailingComments` should not affect `namespace` comments. Anyway, we can fix it in another patch if necessary.


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

https://reviews.llvm.org/D138263