[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-08 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp updated this revision to Diff 487191. carlosgalvezp added a comment. - Add also ImplementationFileExtensions. - Add deprecation notice to affected checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141000/new/

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. > Therefore does it make sense to also do the same with the IncludeStyle as > lots of checks add new includes. Sure, that could also be done, though I think it'd be preferable to do it in a separate patch. Personally I'm a bit reluctant to having formatting

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-06 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. You do raise a good point about duplication, Therefore does it make sense to also do the same with the IncludeStyle as lots of checks add new includes. Though there is a precedent to do away with LLVM/Google style, as clang-format should be responsible for reordering

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. I found now what this global `User` option is about: /// Specifies the name or e-mail of the user running clang-tidy. /// /// This option is used, for example, to place the correct user name in TODO() /// comments in the relevant check. std::optional

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-06 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. On a second thought, using `getLocalOrGlobal` would still require all checks to have 2 private member variables (the raw string option and the parsed header option), together with the logic in the constructor to get the option an parse that. I would like to get

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-05 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. Thanks for the feedback! > Can you explain the reasoning of why this approach is better than current > approach where checks can use global > options(`Options.getLocalOrGlobal("HeaderFileExtensions", > utils::defaultHeaderFileExtensions())`) to access the same

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-05 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment. Can you explain the reasoning of why this approach is better than current approach where checks can use global options(`Options.getLocalOrGlobal("HeaderFileExtensions", utils::defaultHeaderFileExtensions())`) to access the same information? Some checks also have the

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-04 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment. In D141000#4026600 , @Eugene.Zelenko wrote: > Looks OK for me, but probably will be good idea to remove check-specific > options in same commit. Thanks for the review! Sure, I can do that, I just thought we should try to

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-04 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment. Looks OK for me, but probably will be good idea to remove check-specific options in same commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141000/new/ https://reviews.llvm.org/D141000

[PATCH] D141000: [clang-tidy] Introduce HeaderFileExtensions option

2023-01-04 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp created this revision. Herald added subscribers: arphaman, xazax.hun. Herald added a reviewer: njames93. Herald added a project: All. carlosgalvezp requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. We have a number