[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:125
@@ +124,3 @@
+// applying all existing Replaces first if there is conflict.
+void addOrMergeReplacement(const tooling::Replacement &R,
+                           tooling::Replacements *Replaces) {
----------------
alexshap wrote:
> khm, this seems to be a recurring pattern (if i'm not mistaken),
> looks like the interface of tooling::Replacements can be changed/improved (although i don't know).
> Another (separate) observation is about duplicates. For example for replacements in headers we can get into if(Err) branch pretty frequently because the same replacement can be generated multiple times (if that header is included into several *.cpp files). 
SGTM. An `addOrMerge()` interface for tooling::Replacements should be helpful. 

================
Comment at: change-namespace/ChangeNamespace.cpp:481
@@ +480,3 @@
+      continue;
+    }
+    FileToReplacements[FilePath] = *CleanReplacements;
----------------
Added a `FallbackStyle`.

================
Comment at: change-namespace/tool/ClangChangeNamespace.cpp:105
@@ +104,3 @@
+    outs() << "============== " << File << " ==============\n";
+    Rewrite.getEditBuffer(ID).write(llvm::outs());
+    outs() << "\n============================================\n";
----------------
I'd like to keep this for now for better readability. Will change the format in the future if necessary.


https://reviews.llvm.org/D24183