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

Reply via email to