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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG90c5fe98: [include-cleaner] Allow multiple strategies for spelling includes. (authored by VitaNuo). Repository: rG LLVM Github Monorepo CHANG

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

VitaNuo updated this revision to Diff 528341. VitaNuo marked 4 inline comments as done. VitaNuo added a comment. Address the comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-

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

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 ni

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

VitaNuo added a comment. Thanks for the comments! Comment at: clang-tools-extra/clangd/Hover.cpp:1225 - HI.Provider = spellHeader(AST, SM.getFileEntryForID(SM.getMainFileID()), H); + HI.Provider = include_cleaner::spellHeader( + {H, AST.getPreprocessor().getHeaderSearc

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

VitaNuo updated this revision to Diff 528315. VitaNuo marked 11 inline comments as done. VitaNuo added a comment. Herald added subscribers: PiotrZSL, carlosgalvezp. Herald added a reviewer: njames93. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

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

hokein added a comment. thanks, looks good overall, a few more comments Comment at: clang-tools-extra/clangd/Hover.cpp:1225 - HI.Provider = spellHeader(AST, SM.getFileEntryForID(SM.getMainFileID()), H); + HI.Provider = include_cleaner::spellHeader( + {H, AST.getPreproc

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

VitaNuo added a comment. Thanks for the comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

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

VitaNuo updated this revision to Diff 527872. VitaNuo marked 6 inline comments as done. VitaNuo added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-too

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

hokein added a comment. Thanks, a few comments to simplify the code/test further. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:94 +/// shortest spelling. +std::string spellHeader(const IncludeSpellerInput &Input); } // namespace inclu

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

VitaNuo updated this revision to Diff 527466. VitaNuo added a comment. Fix windows test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/cla

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

VitaNuo updated this revision to Diff 527452. VitaNuo added a comment. Remove newline at file end. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-extra/clangd/Hover.cpp clang-tools

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

VitaNuo updated this revision to Diff 527451. VitaNuo added a comment. Remove extra include. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra

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

VitaNuo updated this revision to Diff 527448. VitaNuo added a comment. Add newline at file end. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-ex

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

VitaNuo updated this revision to Diff 527447. VitaNuo added a comment. Add newline at file end. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-ex

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

VitaNuo updated this revision to Diff 527446. VitaNuo added a comment. Fix windows build. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/cl

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

VitaNuo updated this revision to Diff 527438. VitaNuo added a comment. Update. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/clangd/Includ

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

VitaNuo updated this revision to Diff 527435. VitaNuo added a comment. Update. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/clangd/Includ

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

VitaNuo updated this revision to Diff 527420. VitaNuo added a comment. Remove extra path handling. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-extra/clangd/Hover.cpp clang-tools

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

VitaNuo marked 4 inline comments as done. VitaNuo added a comment. Thanks for the comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 ___ cfe-commits mail

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

VitaNuo updated this revision to Diff 527412. VitaNuo added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-ex

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

hokein added a comment. thanks, left some comments per our offline discussion. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:104 +/// order is not specified. +std::function defaultHeaderMapper(); + VitaNuo wrote: > hoke

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

VitaNuo updated this revision to Diff 527330. VitaNuo added a comment. Update test name. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/cla

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

VitaNuo added inline comments. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:88 +class IncludeSpeller { +public: hokein wrote: > hokein wrote: > > I think this is an important API (we will create a subclass for our > >

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

VitaNuo updated this revision to Diff 527314. VitaNuo marked 5 inline comments as done. VitaNuo added a comment. Herald added a subscriber: ormris. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.l

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

hokein added a comment. It looks good overall. I left some comments around the interfaces. Let me know if I miss/misunderstand something. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:88 +class IncludeSpeller { +public: -

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

VitaNuo updated this revision to Diff 526615. VitaNuo added a comment. Remove extra braces. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-extra/clangd/Hover.cpp clang-tools-extra/

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

VitaNuo added a comment. Thanks for the comments! Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:104 + ApplyFirstIncludeSpeller() { +for (const auto &Strategy : + include_cleaner::IncludeSpellingStrategy::entries()) { --

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

VitaNuo updated this revision to Diff 526610. VitaNuo marked 8 inline comments as done. VitaNuo added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-too

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

kadircet added a comment. thanks a lot! outline seems good. mostly some comments on implementation details. Comment at: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h:92 + + virtual std::string operator()(llvm::StringRef HeaderPhysicalPath) const

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

VitaNuo updated this revision to Diff 525581. VitaNuo added a comment. Move the speller out of the loop. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150185/new/ https://reviews.llvm.org/D150185 Files: clang-tools-extra/clangd/Hover.cpp clang

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

VitaNuo created this revision. Herald added subscribers: kadircet, arphaman. Herald added a project: All. VitaNuo updated this revision to Diff 525477. VitaNuo added a comment. VitaNuo updated this revision to Diff 525560. VitaNuo updated this revision to Diff 525566. VitaNuo updated this revision