[PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes
Daniel Jasper via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 11 03:15:55 PDT 2016
djasper added a comment.
Sorry :(... Review is easy enough.. Feel free to ping me more often in the future.
Comment at: lib/Format/WhitespaceManager.cpp:95
@@ -97,2 +94,3 @@
std::sort(Changes.begin(), Changes.end(), Change::IsBeforeInFile(SourceMgr));
> berenm wrote:
> > Maybe we could spare the computation if we aren't going to align anything?
> > Is it better for clarity to always compute additional information? @djasper what's the Clang way to do?
> That's a good point. One certainly could elide that if alignment was turned off. I think so long as it was mentioned in the comments of the ScopeLevel member variable, it would be OK to do so. However, I'll also just defer this decision to @djasper.
Yeah, just avoid unnecessary work.
Comment at: lib/Format/WhitespaceManager.h:173
@@ -165,1 +172,3 @@
+ // down to -1.
+ int ScopeLevel;
NestingLevel does include braces, generally. However, there are two types:
- Braced initializers: These should just work as is.
- Braces that open blocks: Here, child lines are created and so the tokens within a block restart from NestingLevel 0. However, taking that NestingLevel in combination with the Level of the AnnotatedLine should work.
I think reusing that is highly preferable over implementing yet another parentheses counting.
More information about the cfe-commits