[PATCH] D143691: Fix clang-formats IncludeCategory to match the documentation

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks requested changes to this revision.
HazardyKnusperkeks added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/lib/Format/Format.cpp:1379
   LLVMStyle.IncludeStyle.IncludeCategories = {
-      {"^\"(llvm|llvm-c|clang|clang-c)/", 2, 0, false},
-      {"^(<|\"(gtest|gmock|isl|json)/)", 3, 0, false},
-      {".*", 1, 0, false}};
+      {"^\"(llvm|llvm-c|clang|clang-c)/", 2, 2, false},
+      {"^(<|\"(gtest|gmock|isl|json)/)", 3, 3, false},
----------------
I don't follow why this is necessary. And I don't feel comfortable with it.


================
Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:219-221
     if (CategoryRegexs[i].match(IncludeName)) {
-      Ret = Style.IncludeCategories[i].Priority;
-      break;
+      return Style.IncludeCategories[i].Priority;
     }
----------------
Drop the braces (now).


================
Comment at: clang/lib/Tooling/Inclusions/IncludeStyle.cpp:18
     IO &IO, IncludeStyle::IncludeCategory &Category) {
-  IO.mapOptional("Regex", Category.Regex);
-  IO.mapOptional("Priority", Category.Priority);
-  IO.mapOptional("SortPriority", Category.SortPriority);
+  IO.mapRequired("Regex", Category.Regex);
+  IO.mapRequired("Priority", Category.Priority);
----------------
A big no! You will break existing configurations.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143691/new/

https://reviews.llvm.org/D143691