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

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

HazardyKnusperkeks added a comment.

Looks good to me, apart from that one issue.



================
Comment at: clang/lib/Tooling/Inclusions/IncludeStyle.cpp:23
+
+constexpr int DefaultPriority = std::numeric_limits<int>::max();
+
----------------
Please move this into the `mapping` function.


================
Comment at: clang/unittests/Format/SortIncludesTest.cpp:11
 #include "clang/Format/Format.h"
+#include "clang/Tooling/Inclusions/IncludeStyle.h"
 #include "llvm/ADT/StringRef.h"
----------------
Don't need that, or?


================
Comment at: clang/unittests/Format/SortIncludesTest.cpp:548
+  EXPECT_EQ("#include \"a.h\"\n"
+            "#include \"llvm/a.h\"\n"
+            "#include \"b.h\"\n"
----------------
While I may accept there are more than one //main// header, this should not happen in my opinion.


================
Comment at: clang/unittests/Format/SortIncludesTest.cpp:557
+
+  // Keep manually splitted groups in place
   EXPECT_EQ("#include \"a.h\"\n"
----------------
End comments with full stop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143691