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

Febbe added inline comments. Comment at: clang/unittests/Format/SortIncludesTest.cpp:548 + EXPECT_EQ("#include \"a.h\"\n" +"#include \"llvm/a.h\"\n" +"#include \"b.h\"\n" HazardyKnusperkeks wrote: > Febbe wrote: > > HazardyKnusperkeks wro

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

HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/SortIncludesTest.cpp:548 + EXPECT_EQ("#include \"a.h\"\n" +"#include \"llvm/a.h\"\n" +"#include \"b.h\"\n" Febbe wrote: > HazardyKnusperkeks wrote: > > While I m

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

Febbe added inline comments. Comment at: clang/unittests/Format/SortIncludesTest.cpp:548 + EXPECT_EQ("#include \"a.h\"\n" +"#include \"llvm/a.h\"\n" +"#include \"b.h\"\n" HazardyKnusperkeks wrote: > While I may accept there are more than

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

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::max(); + Please move this into the `mapping` function. ==

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

Febbe marked 2 inline comments as done. Febbe added inline comments. 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}, -

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

Febbe updated this revision to Diff 497701. Febbe added a comment. Added requested tests: - since we now support priorities, equal to 0 (the main include priority) sorting should work now for those. - multiple files can be main-includes now, the tests expects that now. - negative priorities do n

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

Febbe updated this revision to Diff 497687. Febbe marked 3 inline comments as done. Febbe added a comment. - Removed some, not required changes. - Reverted mapOptional -> mapRequired change, since it might break existing configs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

HazardyKnusperkeks added inline comments. 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}}; +

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

Febbe added a comment. I've added some elaborations and justifications for the criticized changes. Comment at: clang/lib/Format/Format.cpp:1379 LLVMStyle.IncludeStyle.IncludeCategories = { - {"^\"(llvm|llvm-c|clang|clang-c)/", 2, 0, false}, - {"^(<|\"(gtest|gmock|i

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

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)/",

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

Febbe added a comment. I think I am done, and you can review it now. With the latest changes, SortPriority is no longer required to match Priority, it can always be 0, since Priority is now used in the sorting algorithm. This is what the documentation tends to depict: "and #includes are sorted

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

Febbe updated this revision to Diff 496650. Febbe added a comment. Added priority to the sorting algorithm. This prevents unwanted splits and weird resorts of groups with the same priority, but with different and overlapping (other groups) SortPriority. The new system can now be understood as Pri

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

Febbe updated this revision to Diff 496647. Febbe added a comment. Fixed the Unit Tests - rewritten one test, which made the assumption, that there can be only one main header. - it now asserts, that all matching headers are considered as main header. - replaced initialisations of `IncludeCate

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

Febbe added inline comments. Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:16 #include namespace clang { `` is used, but not included. Currently, it works, since another header already included it. Repository: rG LLVM Github Monorepo CHAN

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

Febbe added a comment. Still some issues with: SortPriorities The following settings (note the SortPriority of '^<.*' == 1) which will produce an extra group for the attached includes: IncludeCategories: - Regex: '^' Priority:2 SortPriority:2 - Regex:

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

Febbe created this revision. Herald added a project: All. Febbe requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. - Main Include Files have now always prio (0,0) - They can't be matched by negative matchers anymore. - `SortPriority` now truly