[PATCH] D21603: [include-fixer] Add missing namespace qualifiers after inserting a missing header.

[Haojian Wu via cfe-commits]

hokein added inline comments.

================
Comment at: include-fixer/IncludeFixer.cpp:234
@@ +233,3 @@
+      std::string MinimizedFilePath = minimizeInclude(
+          ((FilePath[0] == '"' || FilePath[0] == '<') ? FilePath
+                                                      : "\"" + FilePath + "\""),
----------------
djasper wrote:
> How is this change related?
We need to check whether the `FilePath` contains `"` here because the `FilePath` in symbol indexer doesn't contains `<`, but for standard headers, it contains "<".

================
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:88
@@ -86,1 +87,3 @@
 
+cl::opt<bool> FixNamespace("fix-namespace",
+                           cl::desc("Add missing namespace prefix to the "
----------------
djasper wrote:
> Do we really want this? Should we just always do it? Who is going to use this flag?
> 
> If we want it, maybe call it "fix-namespace-qualifiers"? This renaming might also make sense of the patch itself, the test case, ...
I don't have many ideas about this, but I'd like to add this option at the moment, so that user can enable/disable this feature freely. If we want, we could remove it in the future.


http://reviews.llvm.org/D21603