[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

JakeMerdichAMD added a comment. Belated 'sounds good to me'. Other things have been keeping me busy, sorry for the delayed response on this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79388/new/ https://reviews.llvm.org/D79388

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

sylvestre.ledru added a comment. @JakeMerdichAMD as the regression caused by this change is way more significant than the initial bug, I am planning to revert this patch in the next few days (both in master and 11). Please let me know if you have any objection. Repository: rG LLVM Github Mon

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

Abpostelnicu added a comment. In D79388#2321231 , @MyDeveloperDay wrote: > Not to my knowledge, if we are not going to fix it we should probably revert > it for now, we can bring it back later I'm a strong supporter of reverting it for the moment. Rep

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

MyDeveloperDay added a comment. Not to my knowledge, if we are not going to fix it we should probably revert it for now Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79388/new/ https://reviews.llvm.org/D79388 _

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

sylvestre.ledru added a comment. @JakeMerdichAMD @MyDeveloperDay Do you know if the regression has been fixed? thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79388/new/ https://reviews.llvm.org/D79388

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

JakeMerdichAMD added a comment. I'd be very surprised if any of the tests included in this change pass with that line commented it's meant so that things like #if and #else properly separate alignment after the first preprocessor run, where the whitespace manager doesn't have the full conte

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2996 Line->Tokens.back().Tok->MustBreakBefore = true; +Line->Tokens.back().Tok->MustBreakAlignBefore = true; MustBreakBeforeNextToken = false; MyDeveloperDay

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2996 Line->Tokens.back().Tok->MustBreakBefore = true; +Line->Tokens.back().Tok->MustBreakAlignBefore = true; MustBreakBeforeNextToken = false; If the line end

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

MyDeveloperDay added a comment. @JakeMerdichAMD this has caused a regression https://bugs.llvm.org/show_bug.cgi?id=47589 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79388/new/ https://reviews.llvm.org/D79388 ___

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

JakeMerdichAMD added a comment. Thanks for the commit and review @MyDeveloperDay! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79388/new/ https://reviews.llvm.org/D79388 ___ cfe-commits mailing list c

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

This revision was automatically updated to reflect the committed changes. Closed by commit rGb2eb43931757: [clang-format] Fix AlignConsecutive on PP blocks (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79388/new/ https:

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

JakeMerdichAMD added a comment. Hey, @MyDeveloperDay, can I get your assistance in committing this? It's probably been long enough for anyone to chime in. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79388/new/ https://reviews.llvm.org/D79388

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

MyDeveloperDay added a comment. In D79388#2020320 , @JakeMerdichAMD wrote: > Sure, I'll get started on that. It mainly comes from charging headfirst into > the edge cases, but I think I have a decent grasp of clang-format internals > now, and I'm defini

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

JakeMerdichAMD added a comment. Sure, I'll get started on that. It mainly comes from charging headfirst into the edge cases, but I think I have a decent grasp of clang-format internals now, and I'm definitely interested in both contributing to and reviewing for it. Repository: rG LLVM Github

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

MyDeveloperDay added a comment. In D79388#2019985 , @JakeMerdichAMD wrote: > @MyDeveloperDay, you're right about what this issue addresses: it surprised > me a lot when an unrelated edit caused something to 'randomly' add spaces > elsewhere, since it's

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

JakeMerdichAMD added a comment. @MyDeveloperDay, you're right about what this issue addresses: it surprised me a lot when an unrelated edit caused something to 'randomly' add spaces elsewhere, since it's in a different ifdef block (whether or not it has the same condition). The code that's alig

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

MyDeveloperDay accepted this revision. MyDeveloperDay added a subscriber: VelocityRa. MyDeveloperDay added a comment. This revision is now accepted and ready to land. I'm struggling with the simplicity of your solution ;-) (that's a compliment!) I've pulled this patch and built it locally. It ce

[PATCH] D79388: [clang-format] Fix AlignConsecutive on PP blocks

JakeMerdichAMD created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently the 'AlignConsecutive*' options incorrectly align across elif and else statements, even if they are very far away and across unrelated preprocessor macros. This failed since on p