[PATCH] D46496: [Tooling] Pull #include manipulation code from clangFormat into libToolingCore.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

This LG, presuming there are no semantic changes here, just moving the code around.
Also see the nits about `IncludeStyle` that seems to be in the wrong change...



================
Comment at: include/clang/Tooling/Core/HeaderIncludes.h:112
+/// priorities for headers.
+/// FIXME(ioeric): move this class into implementation file when clang-format's
+/// include sorting functions are also moved here.
----------------
ioeric wrote:
> ilya-biryukov wrote:
> > Can we move them in the current commit too?
> > `IncludeCategoryManager` does look like something that should not be in the public header.
> Not yet... include sorting logics would also require refactoring in order to be moved from clang-format.
Ok, a FIXME mentioning that this class should be removed seems good enough. Thanks.


================
Comment at: lib/Format/Format.cpp:1702
   // cases where the first #include is unlikely to be the main header.
-  IncludeCategoryManager Categories(Style, FileName);
+  tooling::IncludeCategoryManager Categories(Style.IncludeStyle, FileName);
   bool FirstIncludeBlock = true;
----------------
It seems `Style.IncludeStyle` should be in the other change that pulls out `IncludeStyle` from `FormatStyle`.
Or am I missing something?


================
Comment at: lib/Format/Format.cpp:1862
   StringRef FileName = Replaces.begin()->getFilePath();
-  HeaderIncludes Includes(FileName, Code, Style);
+  tooling::HeaderIncludes Includes(FileName, Code, Style.IncludeStyle);
 
----------------
Here too. Shouldn't it be part of the other change?


Repository:
  rC Clang

https://reviews.llvm.org/D46496