ioeric added inline comments.
================
Comment at: include-fixer/IncludeFixer.cpp:241
@@ +240,3 @@
+ IncludeFixerContext
+ GetIncludeFixerContext(const clang::SourceManager &SourceManager,
+ clang::HeaderSearch &HeaderSearch) {
----------------
I think function name should start with lower case in llvm.
================
Comment at: include-fixer/IncludeFixer.h:70
@@ +69,3 @@
+/// \return Replacements for inserting and sorting headers.
+std::vector<clang::tooling::Replacement> CreateReplacementsForHeader(
+ StringRef Code, StringRef FilePath, StringRef FallbackStyle,
----------------
Function name should start with lower case.
================
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:211
@@ +210,3 @@
+
+ if (OuputHeaders) {
+ // FIXME: Output IncludeFixerContext as YAML.
----------------
I'd put this before creating replacements and right after you get the context
since those replacements are not used here. And the "Added #include..." message
above doesn't make sense in this mode.
================
Comment at: include-fixer/tool/clang-include-fixer.py:49
@@ +48,3 @@
+ default_choice_index)
+ return int(vim.eval(to_eval));
+
----------------
Maybe handle cases where `to_eval` is not a valid index?
================
Comment at: include-fixer/tool/clang-include-fixer.py:84
@@ +83,3 @@
+ index = 1;
+ for header in lines[1:]:
+ choices_message += "&" + str(index) + header + "\n"
----------------
If there is only one candidate, it doesn't make sense to ask users to choose it
IMO. We can simply insert the header in this case.
And I think we should make users pick the correct header if there are multiple
candidates by default. What do you think, Ben? @bkramer
http://reviews.llvm.org/D20621
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
