[PATCH] D147194: [clang-tidy] fix concat-nest-namespace fix hint remove the macro

[Congcong Cai via Phabricator via cfe-commits]

HerrCai0907 marked 2 inline comments as done.
HerrCai0907 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp:88
+  StringRef TokText = getRawStringRef(TokRange, SM, LangOpts);
+  if (TokText != "// namespace " + ND->getNameAsString())
+    return DefaultSourceRange;
----------------
PiotrZSL wrote:
> PiotrZSL wrote:
> > what if it is //namespace ? or /* namespace XYZ */
> ok, but it still can be things like `//               namespace         XYZ`
> So instead adding single space, consider using some trim/strip.
> but thats minor issues, side effect would be just an leftover namespace comment.
> add some test with something like `// namespace XYZ - closing namespace` to verify that some custom comments wont be removed, and make sure that documentation mention this, that we remove only namespace comments that follow some known standard.
> 
> you could also check for:
> ```
> namespace C
> {
> } // C
> ```
I think it can cover most of scenarios. It don't worth to introduce a mechanism to archive it.
Because something false positive like // name space XYZ should also be considered.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147194