This revision was automatically updated to reflect the committed changes.
Closed by commit rL272669: [clang-format] do not add existing includes.
(authored by ioeric).
Changed prior to commit:
http://reviews.llvm.org/D21323?vs=60680&id=60681#toc
Repository:
rL LLVM
http://reviews.llvm.org/D
djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.
Looks good :-)
http://reviews.llvm.org/D21323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/lis
ioeric added inline comments.
Comment at: unittests/Format/CleanupTest.cpp:701
@@ +700,3 @@
+ std::string Code = "#include \"a.h\"\n#include \n";
+ std::string Expected = "#include \"a.h\"\n#include \"vector\"\n"
+ "#include \n#include \n";
--
ioeric updated this revision to Diff 60680.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- addressed reviewer's comments.
http://reviews.llvm.org/D21323
Files:
lib/Format/Format.cpp
unittests/Format/CleanupTest.cpp
Index: unittests/Format/CleanupTest.cpp
===
djasper added inline comments.
Comment at: unittests/Format/CleanupTest.cpp:693
@@ +692,3 @@
+ std::string Code = "#include \"a.h\"\n#include \n";
+ std::string Expected = "#include \"a.h\"\n#include \n";
+ tooling::Replacements Replaces = {createInsertion("#include "),
---
ioeric updated this revision to Diff 60675.
ioeric marked an inline comment as done.
ioeric added a comment.
- Addressed reviewer's comments: test case added.
http://reviews.llvm.org/D21323
Files:
lib/Format/Format.cpp
unittests/Format/CleanupTest.cpp
Index: unittests/Format/CleanupTest.cp
djasper added a comment.
Lets get this in for now. We can always remove it if we think it is no longer
useful.
Comment at: unittests/Format/CleanupTest.cpp:692
@@ +691,3 @@
+TEST_F(CleanUpReplacementsTest, SkipExistingHeaders) {
+ std::string Code = "#include \"a.h\"\n#include
ioeric added a comment.
Agreed. So, do we still want to skip existing #includes here or simply rely on
`sortIncludes` to de-duplicate?
http://reviews.llvm.org/D21323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-
djasper added a comment.
Well, it is easy to not see duplicated #includes when they aren't properly
sorted. When clang-format then goes in and sorts them, they end up next to each
other. And I think no user really wants that.
http://reviews.llvm.org/D21323
__
ioeric added a comment.
It should be quite straight-forward to de-duplicate #includes in the same block
in `sortIncludes`. I'm happy to add this into `sortIncludes`, but I'm not sure
if this is a behavior users would expect from clang-format since it deletes
code?
And maybe duplicated headers
djasper added a comment.
I am happy to get this in for now, but I actually think the right solution
might be to let clang-format de-duplicate #includes in general. What do you
think?
http://reviews.llvm.org/D21323
___
cfe-commits mailing list
cfe-
ioeric created this revision.
ioeric added a reviewer: djasper.
ioeric added a subscriber: cfe-commits.
Herald added a subscriber: klimek.
do not add existing includes.
http://reviews.llvm.org/D21323
Files:
lib/Format/Format.cpp
unittests/Format/CleanupTest.cpp
Index: unittests/Format/Clean
12 matches
Mail list logo
