[PATCH] D20621: [include-fixer] Create a mode in vim integration to show multiple potential headers.

[Eric Liu via cfe-commits]

ioeric added inline comments.

================
Comment at: include-fixer/IncludeFixer.cpp:241
@@ -280,5 +240,3 @@
   /// \return true if changes will be made, false otherwise.
-  bool Rewrite(clang::SourceManager &SourceManager,
-               clang::HeaderSearch &HeaderSearch,
-               std::set<std::string> &Headers,
-               std::vector<clang::tooling::Replacement> &Replacements) {
+  bool MinimizeAllIncludeHeaders(clang::SourceManager &SourceManager,
+                                 clang::HeaderSearch &HeaderSearch,
----------------
This doesn't seem to be the right name. It does more than minimizing headers.

================
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:121
@@ +120,3 @@
+        clang::include_fixer::CreateReplacementsForHeader(
+            /*Code=*/Buffer.get()->getBuffer(),
+            /*FilePath=*/FilePath,
----------------
These comments seem redundant since it is already clear what those parameters are from variables' names.

================
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:129
@@ +128,3 @@
+        tooling::applyAllReplacements(Buffer.get()->getBuffer(), Replaces);
+    llvm::outs() << ChangedCode;
+    return 0;
----------------
This should only be for vim-mode or STDINMode I think. For normal mode, we should apply replacements on the rewriter.

================
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:208
@@ +207,3 @@
+      clang::include_fixer::CreateReplacementsForHeader(
+          /*Code=*/Buffer.get()->getBuffer(),
+          /*FilePath=*/FilePath,
----------------
Same here.

================
Comment at: include-fixer/tool/clang-include-fixer.py:31
@@ -30,1 +30,3 @@
 
+choosing_mode = False;
+if vim.eval('exists("g:clang_include_fixer_choosing_mode")') == "1":
----------------
I think we can simply have one mode where a header is inserted if there is only one result, and a dialog is shown only if there are multiple headers for user to choose from.

================
Comment at: include-fixer/tool/clang-include-fixer.py:31
@@ -30,1 +30,3 @@
 
+choosing_mode = False;
+if vim.eval('exists("g:clang_include_fixer_choosing_mode")') == "1":
----------------
ioeric wrote:
> I think we can simply have one mode where a header is inserted if there is only one result, and a dialog is shown only if there are multiple headers for user to choose from.
Also remove semicolons here and below...

================
Comment at: include-fixer/tool/clang-include-fixer.py:78
@@ +77,3 @@
+    stdout, stderr = execute(command, text)
+    vim.current.buffer[:] = None;
+    vim.current.buffer.append(stdout.splitlines())
----------------
I think we should also update the buffer with the diff method below. It doesn't make sense to clear the buffer if we are just inserting one line.


http://reviews.llvm.org/D20621