[PATCH] D40288: [clang-format] Add option to group multiple #include blocks when sorting includes

2017-11-27 Thread Krasimir Georgiev via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319024: [clang-format] Add option to group multiple #include 
blocks when sorting… (authored by krasimir).

Changed prior to commit:
  https://reviews.llvm.org/D40288?vs=123924=124361#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40288

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/unittests/Format/SortIncludesTest.cpp

Index: cfe/trunk/unittests/Format/SortIncludesTest.cpp
===
--- cfe/trunk/unittests/Format/SortIncludesTest.cpp
+++ cfe/trunk/unittests/Format/SortIncludesTest.cpp
@@ -77,6 +77,28 @@
   EXPECT_TRUE(sortIncludes(Style, Code, GetCodeRange(Code), "a.cc").empty());
 }
 
+TEST_F(SortIncludesTest, SortedIncludesInMultipleBlocksAreMerged) {
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "\n"
+ "\n"
+ "#include \"b.h\"\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "\n"
+ "\n"
+ "#include \"b.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, SupportClangFormatOff) {
   EXPECT_EQ("#include \n"
 "#include \n"
@@ -159,6 +181,48 @@
  "#include \"b.h\"\n"));
 }
 
+TEST_F(SortIncludesTest, SortsAllBlocksWhenMerging) {
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "\n"
+ "#include \"b.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, CommentsAlwaysSeparateGroups) {
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"c.h\"\n"
+"// comment\n"
+"#include \"b.h\"\n",
+sort("#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "// comment\n"
+ "#include \"b.h\"\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"c.h\"\n"
+"// comment\n"
+"#include \"b.h\"\n",
+sort("#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "// comment\n"
+ "#include \"b.h\"\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"c.h\"\n"
+"// comment\n"
+"#include \"b.h\"\n",
+sort("#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "// comment\n"
+ "#include \"b.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, HandlesAngledIncludesAsSeparateBlocks) {
   EXPECT_EQ("#include \"a.h\"\n"
 "#include \"c.h\"\n"
@@ -180,6 +244,19 @@
  "#include \"a.h\"\n"));
 }
 
+TEST_F(SortIncludesTest, RegroupsAngledIncludesInSeparateBlocks) {
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"c.h\"\n"
+"\n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, HandlesMultilineIncludes) {
   EXPECT_EQ("#include \"a.h\"\n"
 "#include \"b.h\"\n"
@@ -266,6 +343,35 @@
  "a.cc"));
 }
 
+TEST_F(SortIncludesTest, RecognizeMainHeaderInAllGroups) {
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+
+  EXPECT_EQ("#include \"c.h\"\n"
+"#include \"a.h\"\n"
+"#include \"b.h\"\n",
+sort("#include \"b.h\"\n"
+ "\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n",
+ "c.cc"));
+}
+
+TEST_F(SortIncludesTest, MainHeaderIsSeparatedWhenRegroupping) {
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+
+  EXPECT_EQ("#include \"a.h\"\n"
+"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"b.h\"\n"
+ "\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n",
+ "a.cc"));
+}
+
 TEST_F(SortIncludesTest, SupportCaseInsensitiveMatching) {
   // Setup an regex for main includes so we can cover those as well.
   Style.IncludeIsMainRegex =

[PATCH] D40288: [clang-format] Add option to group multiple #include blocks when sorting includes

2017-11-26 Thread Krzysztof Kapusta via Phabricator via cfe-commits 
KrzysztofKapusta added a comment.

@krasimir thanks for the review! I don't have commit rights. Would you push 
this for me?


Repository:
  rL LLVM

https://reviews.llvm.org/D40288



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40288: [clang-format] Add option to group multiple #include blocks when sorting includes

2017-11-24 Thread Krasimir Georgiev via Phabricator via cfe-commits 
krasimir removed a reviewer: krasimir.
krasimir accepted this revision.
krasimir added a comment.
This revision is now accepted and ready to land.

Thank you! Do you have commit access?


Repository:
  rL LLVM

https://reviews.llvm.org/D40288



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40288: [clang-format] Add option to group multiple #include blocks when sorting includes

2017-11-22 Thread Krzysztof Kapusta via Phabricator via cfe-commits 
KrzysztofKapusta added inline comments.



Comment at: unittests/Format/SortIncludesTest.cpp:357
+ "#include \"c.h\"\n",
+ "a.cc"));
+}

krasimir wrote:
> What is this testing?
I changed this to better reflect what I was trying to test. Now the expected 
order is not sorted from a..c 


Repository:
  rL LLVM

https://reviews.llvm.org/D40288



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40288: [clang-format] Add option to group multiple #include blocks when sorting includes

2017-11-22 Thread Krzysztof Kapusta via Phabricator via cfe-commits 
KrzysztofKapusta updated this revision to Diff 123924.
KrzysztofKapusta added a comment.

Addressed all comments. Changed one testcase to better reflect the desired 
effect.


Repository:
  rL LLVM

https://reviews.llvm.org/D40288

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  unittests/Format/SortIncludesTest.cpp

Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -77,6 +77,28 @@
   EXPECT_TRUE(sortIncludes(Style, Code, GetCodeRange(Code), "a.cc").empty());
 }
 
+TEST_F(SortIncludesTest, SortedIncludesInMultipleBlocksAreMerged) {
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "\n"
+ "\n"
+ "#include \"b.h\"\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "\n"
+ "\n"
+ "#include \"b.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, SupportClangFormatOff) {
   EXPECT_EQ("#include \n"
 "#include \n"
@@ -159,6 +181,48 @@
  "#include \"b.h\"\n"));
 }
 
+TEST_F(SortIncludesTest, SortsAllBlocksWhenMerging) {
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "\n"
+ "#include \"b.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, CommentsAlwaysSeparateGroups) {
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"c.h\"\n"
+"// comment\n"
+"#include \"b.h\"\n",
+sort("#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "// comment\n"
+ "#include \"b.h\"\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"c.h\"\n"
+"// comment\n"
+"#include \"b.h\"\n",
+sort("#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "// comment\n"
+ "#include \"b.h\"\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"c.h\"\n"
+"// comment\n"
+"#include \"b.h\"\n",
+sort("#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "// comment\n"
+ "#include \"b.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, HandlesAngledIncludesAsSeparateBlocks) {
   EXPECT_EQ("#include \"a.h\"\n"
 "#include \"c.h\"\n"
@@ -180,6 +244,19 @@
  "#include \"a.h\"\n"));
 }
 
+TEST_F(SortIncludesTest, RegroupsAngledIncludesInSeparateBlocks) {
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"c.h\"\n"
+"\n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, HandlesMultilineIncludes) {
   EXPECT_EQ("#include \"a.h\"\n"
 "#include \"b.h\"\n"
@@ -266,6 +343,35 @@
  "a.cc"));
 }
 
+TEST_F(SortIncludesTest, RecognizeMainHeaderInAllGroups) {
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+
+  EXPECT_EQ("#include \"c.h\"\n"
+"#include \"a.h\"\n"
+"#include \"b.h\"\n",
+sort("#include \"b.h\"\n"
+ "\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n",
+ "c.cc"));
+}
+
+TEST_F(SortIncludesTest, MainHeaderIsSeparatedWhenRegroupping) {
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+
+  EXPECT_EQ("#include \"a.h\"\n"
+"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"b.h\"\n"
+ "\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n",
+ "a.cc"));
+}
+
 TEST_F(SortIncludesTest, SupportCaseInsensitiveMatching) {
   // Setup an regex for main includes so we can cover those as well.
   Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
@@ -309,6 +415,34 @@
  "c_main.cc"));
 }
 
+TEST_F(SortIncludesTest, PriorityGroupsAreSeparatedWhenRegroupping) {
+  Style.IncludeCategories =

[PATCH] D40288: [clang-format] Add option to group multiple #include blocks when sorting includes

2017-11-22 Thread Krasimir Georgiev via Phabricator via cfe-commits 
krasimir added a comment.

Looks good! Just a few nits:




Comment at: lib/Format/Format.cpp:1570
 IncludesInBlock.push_back({IncludeName, Line, Prev, Category});
+  } else if (Style.IncludeBlocks > FormatStyle::IBS_Preserve &&
+ Trimmed.empty()) {

Please replace this with two explicit checks for the expected styles. Plus, an 
`else if` with an empty body is super awkward.



Comment at: unittests/Format/SortIncludesTest.cpp:357
+ "#include \"c.h\"\n",
+ "a.cc"));
+}

What is this testing?


Repository:
  rL LLVM

https://reviews.llvm.org/D40288



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40288: [clang-format] Add option to group multiple #include blocks when sorting includes

2017-11-21 Thread Krzysztof Kapusta via Phabricator via cfe-commits 
KrzysztofKapusta created this revision.
KrzysztofKapusta added a project: clang-tools-extra.
Herald added a subscriber: klimek.

This patch allows grouping multiple #include blocks together and sort all 
includes as one big block.
Additionally, sorted includes can be regrouped after sorting based on 
configured categories.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40288

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  unittests/Format/SortIncludesTest.cpp

Index: unittests/Format/SortIncludesTest.cpp
===
--- unittests/Format/SortIncludesTest.cpp
+++ unittests/Format/SortIncludesTest.cpp
@@ -77,6 +77,28 @@
   EXPECT_TRUE(sortIncludes(Style, Code, GetCodeRange(Code), "a.cc").empty());
 }
 
+TEST_F(SortIncludesTest, SortedIncludesInMultipleBlocksAreMerged) {
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "\n"
+ "\n"
+ "#include \"b.h\"\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "\n"
+ "\n"
+ "#include \"b.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, SupportClangFormatOff) {
   EXPECT_EQ("#include \n"
 "#include \n"
@@ -159,6 +181,48 @@
  "#include \"b.h\"\n"));
 }
 
+TEST_F(SortIncludesTest, SortsAllBlocksWhenMerging) {
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"a.h\"\n"
+ "#include \"c.h\"\n"
+ "\n"
+ "#include \"b.h\"\n"));
+}
+
+TEST_F(SortIncludesTest, CommentsAlwaysSeparateGroups) {
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"c.h\"\n"
+"// comment\n"
+"#include \"b.h\"\n",
+sort("#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "// comment\n"
+ "#include \"b.h\"\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"c.h\"\n"
+"// comment\n"
+"#include \"b.h\"\n",
+sort("#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "// comment\n"
+ "#include \"b.h\"\n"));
+
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"c.h\"\n"
+"// comment\n"
+"#include \"b.h\"\n",
+sort("#include \"c.h\"\n"
+ "#include \"a.h\"\n"
+ "// comment\n"
+ "#include \"b.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, HandlesAngledIncludesAsSeparateBlocks) {
   EXPECT_EQ("#include \"a.h\"\n"
 "#include \"c.h\"\n"
@@ -180,6 +244,19 @@
  "#include \"a.h\"\n"));
 }
 
+TEST_F(SortIncludesTest, RegroupsAngledIncludesInSeparateBlocks) {
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"c.h\"\n"
+"\n"
+"#include \n"
+"#include \n",
+sort("#include \n"
+ "#include \n"
+ "#include \"c.h\"\n"
+ "#include \"a.h\"\n"));
+}
+
 TEST_F(SortIncludesTest, HandlesMultilineIncludes) {
   EXPECT_EQ("#include \"a.h\"\n"
 "#include \"b.h\"\n"
@@ -266,6 +343,35 @@
  "a.cc"));
 }
 
+TEST_F(SortIncludesTest, RecognizeMainHeaderInAllGroups) {
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  Style.IncludeBlocks = FormatStyle::IBS_Merge;
+
+  EXPECT_EQ("#include \"a.h\"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"b.h\"\n"
+ "\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n",
+ "a.cc"));
+}
+
+TEST_F(SortIncludesTest, MainHeaderIsSeparatedWhenRegroupping) {
+  Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
+  Style.IncludeBlocks = FormatStyle::IBS_Regroup;
+
+  EXPECT_EQ("#include \"a.h\"\n"
+"\n"
+"#include \"b.h\"\n"
+"#include \"c.h\"\n",
+sort("#include \"b.h\"\n"
+ "\n"
+ "#include \"a.h\"\n"
+ "#include \"c.h\"\n",
+ "a.cc"));
+}
+
 TEST_F(SortIncludesTest, SupportCaseInsensitiveMatching) {
   // Setup an regex for main includes so we can cover those as well.
   Style.IncludeIsMainRegex = "([-_](test|unittest))?$";
@@