[PATCH] D24183: A clang tool for changing surrouding namespaces of class/function definitions.

[Eric Liu via cfe-commits]

ioeric added inline comments.

================
Comment at: change-namespace/ChangeNamespace.cpp:200
@@ +199,3 @@
+  while (!NsSplitted.empty()) {
+    // FIXME: consider code style for comments.
+    Code = ("namespace " + NsSplitted.back() + " {\n" + Code +
----------------
hokein wrote:
> Doesn't `formatAndApplyAllReplacements` format the comments for us?
formatter only adds/removes white spaces I think. 

================
Comment at: change-namespace/ChangeNamespace.cpp:400
@@ +399,3 @@
+  assert(Consumed && "Expect OldNS to start with OldNamespace.");
+  (void)Consumed;
+  const std::string NewNs = (NewNamespace + Postfix).str();
----------------
hokein wrote:
> how about `assert(Postfix.consume_front(OldNamespace) && "Expect OldNS to start with OldNamespace.");`?
The problem with this is that `Postfix.consume_front(OldNamespace)` won't be executed if assertion is turned off.


https://reviews.llvm.org/D24183