[PATCH] D154963: [Format][Tooling] Fix HeaderIncludes::insert not respect the main-file header.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:338
     int Priority = Categories.getIncludePriority(
-        CurInclude.Name, /*CheckMainHeader=*/FirstIncludeOffset < 0);
+        CurInclude.Name, /*CheckMainHeader=*/true);
     CategoryEndOffsets[Priority] = NextLineOffset;
----------------
instead of doing this unconditionally, can we do this until we find first include that looks like the main include? similarly we should also only look for main includes during `::insert` if we didn't encounter one already during initial scan.

this ensures we give users a way out (in certain cases) when there are multiple alternatives, by respecting the order they have. it's also aligned with the behaviour of `sortCppIncludes`, which is used when formatting files elsewhere.


================
Comment at: clang/unittests/Tooling/HeaderIncludesTest.cpp:146
 
+TEST_F(HeaderIncludesTest, InsertMainHeader) {
+  std::string Code = R"cpp(#include "a.h")cpp";
----------------
can you also have test cases with competing includes, eg:

foo.cc, with contents:
```
#include "zoo/foo.h"
// insert "xoo/foo.h"
```
-> xoo/foo.h doesn't become main include


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154963