[PATCH] D150185: [include-cleaner] Allow multiple strategies for spelling includes.

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

Thanks, looks great!



================
Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/IncludeSpeller.h:22
+
+class IncludeSpeller {
+
----------------
would be nice to add some documentation for this interface, something like

```
IncludeSpeller provides an extension point to allow clients implement custom include spelling strategies applications for physical headers.
```


================
Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/IncludeSpeller.h:23
+class IncludeSpeller {
+
+public:
----------------
nit: remove the extra blank line.


================
Comment at: clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp:24
+class DefaultIncludeSpeller : public IncludeSpeller {
+
+  std::string operator()(const Input &Input) const override {
----------------
add a `public:`.


================
Comment at: clang-tools-extra/include-cleaner/lib/IncludeSpeller.cpp:33
+
+std::string mapHeader(const IncludeSpeller::Input &Input) {
+  static auto Spellers = [] {
----------------
nit: maybe name it `spellPhysicalHeader`( I'd avoid using `map`, it reminds me too much on the stdlib mapping).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150185