[PATCH] D88296: [clang-format] De-duplicate includes with leading or trailing whitespace.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfe21c86ee75f: [clang-format] De-duplicate includes with
leading or trailing whitespace. (authored by curdeius).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88296/new/
https://reviews.llvm.org/D88296
Files:
clang/lib/Format/Format.cpp
clang/unittests/Format/SortIncludesTest.cpp
Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -289,6 +289,19 @@
sort("# include \"a.h\"\n"
"# include \"c.h\"\n"
"# include \"b.h\"\n"));
+ EXPECT_EQ("#include \"a.h\"\n", sort("#include \"a.h\"\n"
+ " #include \"a.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, TrailingWhitespace) {
+ EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"a.h\" \n"
+ "#include \"c.h\" \n"
+ "#include \"b.h\" \n"));
+ EXPECT_EQ("#include \"a.h\"\n", sort("#include \"a.h\"\n"
+ "#include \"a.h\" \n"));
}
TEST_F(SortIncludesTest, GreaterInComment) {
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2179,7 +2179,8 @@
// Deduplicate #includes.
Indices.erase(std::unique(Indices.begin(), Indices.end(),
[&](unsigned LHSI, unsigned RHSI) {
- return Includes[LHSI].Text ==
Includes[RHSI].Text;
+ return Includes[LHSI].Text.trim() ==
+ Includes[RHSI].Text.trim();
}),
Indices.end());
Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -289,6 +289,19 @@
sort("# include \"a.h\"\n"
"# include \"c.h\"\n"
"# include \"b.h\"\n"));
+ EXPECT_EQ("#include \"a.h\"\n", sort("#include \"a.h\"\n"
+ " #include \"a.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, TrailingWhitespace) {
+ EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"a.h\" \n"
+ "#include \"c.h\" \n"
+ "#include \"b.h\" \n"));
+ EXPECT_EQ("#include \"a.h\"\n", sort("#include \"a.h\"\n"
+ "#include \"a.h\" \n"));
}
TEST_F(SortIncludesTest, GreaterInComment) {
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2179,7 +2179,8 @@
// Deduplicate #includes.
Indices.erase(std::unique(Indices.begin(), Indices.end(),
[&](unsigned LHSI, unsigned RHSI) {
- return Includes[LHSI].Text == Includes[RHSI].Text;
+ return Includes[LHSI].Text.trim() ==
+ Includes[RHSI].Text.trim();
}),
Indices.end());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D88296: [clang-format] De-duplicate includes with leading or trailing whitespace.
curdeius added a comment. Thanks for the review. I agree with you that a clang-format user should not expect a perfectly formatted code after a single iteration. Maybe it's actually a documentation issue and we should be clear about it? Just a guarantee of 1) stability of formatting and 2) asymptotically arriving at the fully formatted code. I'll think about how to word it and create a revision for the documentation (unless you judge it unnecessary). BTW, I would be almost inclined to abandon this review and mark the bug report as invalid... but the change is so trivial... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88296/new/ https://reviews.llvm.org/D88296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D88296: [clang-format] De-duplicate includes with leading or trailing whitespace.
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. This LGTM, generally I think clang-format seems to remove trailing spaces anyway so it might be that the duplicate might disappear on the second format. but I'm good with this approach But as a side note I keep wondering who are these people who clang-format once and expect it to be perfect! Firstly I'm clang-format -n checking my 4-5 million lines of code every night, secondly the CI system is checking, and thirdly in our company we using clang-format on save, and Ctrl-S and :w are like a tick for me. By the time I check my code in I've clang-formatted it 100's of times. I don't even need to use git clang-format I know its perfect already! This is in my view the resolution to pretty much all those, I have to do it twice! bugs..its a non issue for anyone with a zero tolerance policy to un-clang-formtted code! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88296/new/ https://reviews.llvm.org/D88296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D88296: [clang-format] De-duplicate includes with leading or trailing whitespace.
curdeius added a comment. Finally I've opted for creating a small revision just fixing this particular bug report. Mind however that the problem is a bit more complex and to solve all possible cases we would need to first reformat the source code, then sort the includes and then again, possibly reformat parts of the source code modified by sorting, so that the comments get re-aligned etc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88296/new/ https://reviews.llvm.org/D88296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D88296: [clang-format] De-duplicate includes with leading or trailing whitespace.
curdeius updated this revision to Diff 294291.
curdeius added a comment.
- Ooops. Revert unwanted test changes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88296/new/
https://reviews.llvm.org/D88296
Files:
clang/lib/Format/Format.cpp
clang/unittests/Format/SortIncludesTest.cpp
Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -272,6 +272,19 @@
sort("# include \"a.h\"\n"
"# include \"c.h\"\n"
"# include \"b.h\"\n"));
+ EXPECT_EQ("#include \"a.h\"\n", sort("#include \"a.h\"\n"
+ " #include \"a.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, TrailingWhitespace) {
+ EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"a.h\" \n"
+ "#include \"c.h\" \n"
+ "#include \"b.h\" \n"));
+ EXPECT_EQ("#include \"a.h\"\n", sort("#include \"a.h\"\n"
+ "#include \"a.h\" \n"));
}
TEST_F(SortIncludesTest, GreaterInComment) {
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2160,7 +2160,8 @@
// Deduplicate #includes.
Indices.erase(std::unique(Indices.begin(), Indices.end(),
[&](unsigned LHSI, unsigned RHSI) {
- return Includes[LHSI].Text ==
Includes[RHSI].Text;
+ return Includes[LHSI].Text.trim() ==
+ Includes[RHSI].Text.trim();
}),
Indices.end());
Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -272,6 +272,19 @@
sort("# include \"a.h\"\n"
"# include \"c.h\"\n"
"# include \"b.h\"\n"));
+ EXPECT_EQ("#include \"a.h\"\n", sort("#include \"a.h\"\n"
+ " #include \"a.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, TrailingWhitespace) {
+ EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"a.h\" \n"
+ "#include \"c.h\" \n"
+ "#include \"b.h\" \n"));
+ EXPECT_EQ("#include \"a.h\"\n", sort("#include \"a.h\"\n"
+ "#include \"a.h\" \n"));
}
TEST_F(SortIncludesTest, GreaterInComment) {
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2160,7 +2160,8 @@
// Deduplicate #includes.
Indices.erase(std::unique(Indices.begin(), Indices.end(),
[&](unsigned LHSI, unsigned RHSI) {
- return Includes[LHSI].Text == Includes[RHSI].Text;
+ return Includes[LHSI].Text.trim() ==
+ Includes[RHSI].Text.trim();
}),
Indices.end());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D88296: [clang-format] De-duplicate includes with leading or trailing whitespace.
curdeius created this revision.
curdeius added a reviewer: MyDeveloperDay.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
curdeius requested review of this revision.
This fixes PR46555.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D88296
Files:
clang/lib/Format/Format.cpp
clang/unittests/Format/SortIncludesTest.cpp
Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -266,12 +266,19 @@
sort(" #include \"a.h\"\n"
" #include \"c.h\"\n"
" #include \"b.h\"\n"));
+ EXPECT_EQ("#include \"a.h\"\n", sort("#include \"a.h\"\n"
+ " #include \"a.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, TrailingWhitespace) {
EXPECT_EQ("#include \"a.h\"\n"
"#include \"b.h\"\n"
"#include \"c.h\"\n",
-sort("# include \"a.h\"\n"
- "# include \"c.h\"\n"
- "# include \"b.h\"\n"));
+sort("#include \"a.h\" \n"
+ "#include \"c.h\" \n"
+ "#include \"b.h\" \n"));
+ EXPECT_EQ("#include \"a.h\"\n", sort("#include \"a.h\"\n"
+ "#include \"a.h\" \n"));
}
TEST_F(SortIncludesTest, GreaterInComment) {
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2160,7 +2160,8 @@
// Deduplicate #includes.
Indices.erase(std::unique(Indices.begin(), Indices.end(),
[&](unsigned LHSI, unsigned RHSI) {
- return Includes[LHSI].Text ==
Includes[RHSI].Text;
+ return Includes[LHSI].Text.trim() ==
+ Includes[RHSI].Text.trim();
}),
Indices.end());
Index: clang/unittests/Format/SortIncludesTest.cpp
===
--- clang/unittests/Format/SortIncludesTest.cpp
+++ clang/unittests/Format/SortIncludesTest.cpp
@@ -266,12 +266,19 @@
sort(" #include \"a.h\"\n"
" #include \"c.h\"\n"
" #include \"b.h\"\n"));
+ EXPECT_EQ("#include \"a.h\"\n", sort("#include \"a.h\"\n"
+ " #include \"a.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, TrailingWhitespace) {
EXPECT_EQ("#include \"a.h\"\n"
"#include \"b.h\"\n"
"#include \"c.h\"\n",
-sort("# include \"a.h\"\n"
- "# include \"c.h\"\n"
- "# include \"b.h\"\n"));
+sort("#include \"a.h\" \n"
+ "#include \"c.h\" \n"
+ "#include \"b.h\" \n"));
+ EXPECT_EQ("#include \"a.h\"\n", sort("#include \"a.h\"\n"
+ "#include \"a.h\" \n"));
}
TEST_F(SortIncludesTest, GreaterInComment) {
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2160,7 +2160,8 @@
// Deduplicate #includes.
Indices.erase(std::unique(Indices.begin(), Indices.end(),
[&](unsigned LHSI, unsigned RHSI) {
- return Includes[LHSI].Text == Includes[RHSI].Text;
+ return Includes[LHSI].Text.trim() ==
+ Includes[RHSI].Text.trim();
}),
Indices.end());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
