Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper added a comment. So.. I finally got some time to look at this again: Quick Recap - IndentLevel and NestingLevel are now stored separately inside WhitespaceManager::Change. I've added a function ScopeLevel() which combines them with a bit of logic, and returns a number that can be used

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper updated this revision to Diff 72444. bmharper added a comment. This revision merges NestingLevel and IndentLevel into an std::pair, as suggested by @djasper. IMO this makes the code slightly harder to read, because you lose the unique variable names "IndentLevel" and "NestingLevel". Tho

[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper added a comment. ping! https://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper removed rL LLVM as the repository for this revision. bmharper updated this revision to Diff 68675. bmharper added a comment. Fixed the two issues pointed out by djasper in his most recent comments: 1. Only calculate ScopeLevel when necessary. 2. Instead of calculating ScopeLevel by inspe

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper added a comment. The reason one has to precompute ScopeLevel is because IndentLevel is not actually meaningful on each token. It's only meaningful for the first token on the line (the remaining tokens on the line have IndentLevel = 0). So if you look at the implementation of calculateS

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper added a comment. I'll see what happens if I make IndentLevel the same for all tokens on a line. If not too much is broken by that, I should be able to handle it. https://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@list

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper updated this revision to Diff 69671. bmharper added a comment. I have added an initial phase which propagates IndentLevel from the first token on a line, to all of the tokens on that line. This allows us to get rid of the ScopeLevel state. However, I have retained the name "ScopeLevel",

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper added a comment. PING! My previous commit hopefully addressed the issue with the sprawl of IndentLevel + ScopeLevel https://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper added a comment. PING Comment at: lib/Format/WhitespaceManager.cpp:95 @@ -97,2 +94,3 @@ std::sort(Changes.begin(), Changes.end(), Change::IsBeforeInFile(SourceMgr)); + calculateScopeLevel(); calculateLineBreakInformation(); berenm wrote: > Maybe

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

PING. Is there anything I can do to make reviewing easier? For example, I could run my code vs master on some large code bases (eg linux kernel, chromium), and verify that there are no crashes, and also manually spot check some results. Ben On Wed, 20 Jul 2016 at 21:43 Ben Harper wrote: > bmh

[clang-format] recursive alignment

I'm calling this "recursive alignment", but what it really fixes is issues with AlignConsecutiveAssignments and AlignConsecutiveDeclarations such as this: OLD: int fun1(int a); double fun2(int b); NEW intfun1(int a); double fun2(int b); Also... OLD: void fun(int x = 1) { int

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper added a comment. As requested - full diff (of all three files) F2059068: AlignFullDiff.patch Repository: rL LLVM http://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper removed rL LLVM as the repository for this revision. bmharper updated this revision to Diff 60569. bmharper added a comment. diff with full context http://reviews.llvm.org/D21279 Files: lib/Format/WhitespaceManager.cpp lib/Format/WhitespaceManager.h unittests/Format/FormatTest.cp

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper added a comment. Interesting. Working on it! http://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper set the repository for this revision to rL LLVM. bmharper updated this revision to Diff 60830. bmharper added a comment. Fix the recent two issues mentioned by Beren, ie the single-statement scopes (for loop without braces), and operator[] alignment. Repository: rL LLVM http://revie

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper added a comment. I've taken some time to investigate those two issues, and these are my thoughts: 1. Constructor alignment: I think this is a good thing to do, but if `isFunctionDeclarationName`, and it's caller `TokenAnnotator::calculateFormattingInformation` are anything to go by, ad

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper added a subscriber: bmharper. bmharper added a comment. Hi Daniel, Is there anything else that I need to do here? Regards, Ben Repository: rL LLVM http://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper added a comment. Friendly PING. Please let me know if there's anything else that I need to do here, otherwise I'll keep quiet and expect a merge at some point? Repository: rL LLVM http://reviews.llvm.org/D21279 ___ cfe-commits mailing l

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

bmharper added a comment. kaPING! Repository: rL LLVM http://reviews.llvm.org/D21279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits