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

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

The moving of IncludeStyle has been split into a separate patch https://reviews.llvm.org/D46758.



================
Comment at: include/clang/Format/Format.h:1818
 ///   from \p Code if it exists.
+/// See documentation for `tooling::HeaderIncludes` for the behavior.
 llvm::Expected<tooling::Replacements>
----------------
ilya-biryukov wrote:
> `for the behavior` is a little confusing. Could we try elaborating on what 'behavior' is a bit more? E.g. `the include manipulation is done via tooling::HeaderInclude, see its documentation for more details on how include insertion points are found and which edits are produced`.
Sure. Thanks for the suggestion!


================
Comment at: include/clang/Tooling/Core/HeaderIncludes.h:22
+
+struct IncludeStyle {
+  /// Styles for sorting multiple ``#include`` blocks.
----------------
ilya-biryukov wrote:
> This struct looks big enough to justify putting it into a separate header. For the sake of keeping `HeaderIncludes` the first thing that people see when they open a file.
> WDYT?
Good point. Branched into D46758


================
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.
----------------
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.


Repository:
  rC Clang

https://reviews.llvm.org/D46496