hokein added a comment.
In http://reviews.llvm.org/D20621#444050, @bkramer wrote:
> LG. Can't wait to use it myself :)
Currently, the header is only inserted at the first line of the file because we
don't output the FirstIncludeOffset to py script. A follow-up patch will come
soon.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL271258: [include-fixer] Create a mode in vim integration to
show multiple potential… (authored by hokein).
Changed prior to commit:
http://reviews.llvm.org/D20621?vs=59028=59030#toc
Repository:
rL
klimek accepted this revision.
klimek added a reviewer: klimek.
klimek added a comment.
lg
http://reviews.llvm.org/D20621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hokein marked an inline comment as done.
hokein added a comment.
http://reviews.llvm.org/D20621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hokein updated this revision to Diff 59028.
hokein added a comment.
Add -1U comment back.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
klimek added inline comments.
Comment at: include-fixer/IncludeFixer.h:74
@@ +73,3 @@
+/// \return Replacements for inserting and sorting headers.
+std::vector createInsertHeaderReplacements(
+StringRef Code, StringRef FilePath, StringRef Header,
This is
hokein updated this revision to Diff 59026.
hokein added a comment.
Update a out-of-date comment.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
bkramer accepted this revision.
bkramer added a comment.
This revision is now accepted and ready to land.
LG. Can't wait to use it myself :)
http://reviews.llvm.org/D20621
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
hokein updated this revision to Diff 59018.
hokein marked 2 inline comments as done.
hokein added a comment.
Fix code style.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
bkramer added inline comments.
Comment at: include-fixer/IncludeFixer.h:71
@@ +70,3 @@
+/// \param FirstIncludeOffset The insertion point for new include directives.
+/// -1U(UINT_MAX) indicates uninitialization, which will insert the header at
+/// first line. If there is no
hokein added inline comments.
Comment at: include-fixer/IncludeFixer.h:80
@@ +79,3 @@
+unsigned FirstIncludeOffset=-1U,
+const clang::format::FormatStyle =clang::format::getLLVMStyle());
+
Using a default argument in `Style` can simplify the code in some
hokein updated this revision to Diff 58964.
hokein marked an inline comment as done.
hokein added a comment.
Use format::getStyle to get clang-format style.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
ioeric added inline comments.
Comment at: include-fixer/IncludeFixer.h:80
@@ +79,3 @@
+unsigned FirstIncludeOffset=-1U,
+const clang::format::FormatStyle =clang::format::getLLVMStyle());
+
I don't see why we'd want Style to be optional.
hokein updated this revision to Diff 58955.
hokein marked an inline comment as done.
hokein added a comment.
Refactor createReplacementsForHeaders.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
klimek added a subscriber: klimek.
Comment at: include-fixer/IncludeFixer.cpp:397
@@ +396,3 @@
+ clang::tooling::Replacements Insertions;
+ if (FirstIncludeOffset == -1U) {
+// FIXME: skip header guards.
Do we want UINT_MAX? I find the wording in the
hokein updated this revision to Diff 58942.
hokein added a comment.
Remove unneeded headers.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
hokein updated this revision to Diff 58941.
hokein marked 9 inline comments as done.
hokein added a comment.
Address comments.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
hokein added a comment.
In http://reviews.llvm.org/D20621#439447, @bkramer wrote:
> Can you add some lit tests for the various command line modes
> clang-include-fixer has now. We can't reasonably test the vim integration but
> we can tests the bits it's composed of.
Done.
bkramer added inline comments.
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"
ioeric wrote:
> If there is only one candidate, it doesn't
ioeric added inline comments.
Comment at: include-fixer/IncludeFixer.cpp:241
@@ +240,3 @@
+ IncludeFixerContext
+ GetIncludeFixerContext(const clang::SourceManager ,
+ clang::HeaderSearch ) {
I think function name should start with lower
bkramer added inline comments.
Comment at: include-fixer/IncludeFixer.h:51
@@ -50,6 +50,3 @@
- /// Headers to be added.
- std::set
-
- /// Replacements are written here.
- std::vector
+ /// The context that contains all information about queried symbol.
+
bkramer added a comment.
Can you add some lit tests for the various command line modes
clang-include-fixer has now. We can't reasonably test the vim integration but
we can tests the bits it's composed of.
http://reviews.llvm.org/D20621
___
hokein updated this revision to Diff 58448.
hokein marked 6 inline comments as done.
hokein added a comment.
Update and address comments.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
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 ,
- clang::HeaderSearch ,
- std::set ,
-
hokein updated this revision to Diff 58420.
hokein added a comment.
Fix a nit.
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
hokein updated this revision to Diff 58415.
hokein added a comment.
Rebase
http://reviews.llvm.org/D20621
Files:
include-fixer/IncludeFixer.cpp
include-fixer/IncludeFixer.h
include-fixer/IncludeFixerContext.h
include-fixer/tool/ClangIncludeFixer.cpp
hokein added a comment.
Oh, sorry, I miss two separate commits here. This patch should not be ready
for review. I need to rebase it after commit http://reviews.llvm.org/D20581.
http://reviews.llvm.org/D20621
___
cfe-commits mailing list
hokein created this revision.
hokein added a reviewer: bkramer.
hokein added subscribers: ioeric, cfe-commits.
Some changes in the patch:
* Add two commandline flags in clang-include-fixer.
* Introduce a IncludeFixerContext for the queried symbol.
* Pull out CreateReplacementsForHeader.
28 matches
Mail list logo
