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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits