[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

2023-04-19 Thread Fabian Keßler via Phabricator via cfe-commits
Febbe added a comment. Actually, I already wanted to add an enum, controlling this behavior. But even with an enum, this check should only remove includes in consecutive blocks, which could be relocated (are not split by `#defines` or a comment) Or I should add the options to

[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

2023-04-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > Regarding the comment that I must not change existing tests: I think this > rule is too strict, because those tests are mostly regression tests. > But a regression tests does not test for correctness. So if a test had > already a wrong assumption, it must be

[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

2023-04-17 Thread Fabian Keßler via Phabricator via cfe-commits
Febbe added a comment. In D143870#4273075 , @MyDeveloperDay wrote: > I know it might not seem an obvious use case but there really isn't a > requirement to not include header files more than once.. imagine if I have > > #define ARCH "win32" >

[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

2023-04-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. I know it might not seem an obvious use case but there really isn't a requirement to not include header files more than once.. imaging if I have #define ARCH "win32" #include

[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

2023-02-14 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D143870#4124057 , @Febbe wrote: > @HazardyKnusperkeks thank you for the review, I would add another option, but > I don't know a good name. I would propose a > > `boolean` `IncludeDeduplicateInAllBlocks` which

[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

2023-02-13 Thread Fabian Keßler via Phabricator via cfe-commits
Febbe added a comment. @HazardyKnusperkeks thank you for the review, I would add another option, but I don't know a good name. I would propose a `boolean` `IncludeDeduplicateInAllBlocks` which defaults to zero. First an `Include`, to keep include-sorting related options together in the

[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

2023-02-13 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added a comment. This revision now requires changes to proceed. I can see that this is maybe useful, but that have to be behind a new option, which is turned off by default. And a big no to changing the existing tests,

[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

2023-02-12 Thread Fabian Keßler via Phabricator via cfe-commits
Febbe created this revision. Herald added a project: All. Febbe requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This change aims to remove all duplicate include directives, instead of only those, which are in the same block. This change