- User Since
- Jun 27 2015, 11:40 AM (306 w, 2 d)
Dec 8 2017
Yes, please could you commit it for me? Thanks!
Rebase patch on upstream HEAD
Dec 7 2017
Factor !Type.isNull() && Type.isConstQualified() condition
Oct 27 2017
Mar 14 2017
Jan 9 2017
There's https://reviews.llvm.org/D27651 that will conflict with this one.
Yes you are right,
I'm trying to think of a scenario where *, && or & before tokens to be aligned would not indicate pointers or references, but as the alignment is only done for now on declarations and assignments, I can't find one.
Sep 23 2016
This patch might help clarifying the NestingLevel usage and can simplify the alignment computations in https://reviews.llvm.org/D21279
I had a little bit of look into the NestingLevel field. I understand that it only indicates the nesting level of the token inside the current AnnotatedLine, which could very well be the same as the nesting level of another token in the previous or next AnnotatedLine. Right now, it doesn't include the information on the nested level of the AnnotatedLine itself (this is in the Level field of the line itself).
Aug 27 2016
Jun 21 2016
Actually it looks like it works now... I'm not sure what I did when I had this misalignment. It would have been fine by me anyway.
Jun 20 2016
Thanks! The operators are now correctly handled.
Jun 14 2016
It looks like it doesn't like the operator either:
Jun 13 2016
There's still cases where the nesting level is still not correctly handled: when using unbraced conditionals / loops. For example (sorry, silly example):
I think it's better to update the diff with full context (git diff -U999999) rather than attach the file, in order to have it integrated into phabricator diff view. Or use the arcanist command-line tool, which should do it for you.
Jan 15 2016
Dec 22 2015
Actually, the main issue that I see is the difficulty to build the plugin itself. The project does not build out-of-the-box on VS2013, and I had to change the assembly references by hand.
For information, I just tested the current diff under Visual Studio 2010 SP1 and everything looks fine as well.
Dec 14 2015
- Patch rebased on trunk
- Small cleanups in the RunningDocumentTable usage
- Successfully tested on VS >= 2012
Dec 1 2015
I don't have any compilation error with GCC 5.2.1. I tried again with Clang 3.8 and indeed there is this error:
Nov 28 2015
I don't have commit access, if you don't mind landing it for me :)
Nov 27 2015
Fix continue statements.
[clang-format] Code cleanup and variable naming in WhitespaceManager::AlignTokens
Nov 26 2015
Updated the diff with a fix for the issue reported in http://lists.llvm.org/pipermail/cfe-dev/2015-November/046057.html
[clang-format] alignConsecutiveXXX: replace the buggy paren / braces counter with a better scope tracker.
Nov 18 2015
Nov 6 2015
Nov 4 2015
[clang-format] Replace TokenSequenceAligner with simple static functions.
[clang-format] Remove deleted methods declaration.
[clang-format] Alignment code factorization.
I've also added a fix for the other issue reported on the same bug. I could split the two reviews if necessary.
[clang-format] Count the number of braces and parens on the line instead of remembering only one.
Oct 7 2015
Oct 1 2015
Thanks, I don't have commit access.
Fix arcanist insisting on creating the diff against old revision...
- Add support of ColumnLimit restrition in AlignConsecutiveAssignments and AlignConsecutiveDeclarations.
Alright, thank you for your time!
Sep 29 2015
Fix arcanist messup...
Address the latest comments, as well as useless code removal.
- Only the start of the token range is stored, so there is no need to care about the end of the range. Remove the code that was trying to match the end of the range (template, nested name).
- Add some spaces in test cases (destructors, templates) just to be sure that everything works as expected.
- Add a missing CHECK-FIXES in the test case on the abstract_class destructor.
Update the diff with the ProvideAutoLoad attribute.
Sep 27 2015
Actually I believe it's better fixed by D13081.
I think the latest version also addresses D13090 in a better way. The check will now let clang-tidy decide whether warnings and fixes are displayed, whether it is in or from system/user header files or in main file, and will only worry about not emitting warnings and fixes if a macro is involved in the declaration or any usage.
Reworked the patches a little bit, addressing the comments and adding
better handling of checks in header files with corresponding unit test cases.
Yes please, I cannot commit it myself.
Added comments and reformated NamingCheckIdentifer.h with clang-format.
Sep 24 2015
- Do not check for identifier names from system headers
- Check for SourceLocation validity before modifying them
This will also disable all warnings for declaration / usages outside of the main file.
Reformatting with clang-format
Update the diff with more context.
Update the diff with more context, thanks to arcanist.
Changed Usages to RawUsageLocs, as DenseSet storing the raw encoding of SourceLocation instead of a vector or SourceRange.
Well, the format on save setting is disabled by default, do you mean you had to enable it twice? Or you had it enabled without the C++ development tools, and after the installation you had to disable and enable it again?
Sep 23 2015
Ping? Just to be sure it's not going to fall to oblivion. :)
Even better with the corresponding unit test fix.
Sep 22 2015
Fix incorrect line removal in unit test.
@jbcoe Sorry for making this patch irrelevant but I managed to have the generic approach working in http://reviews.llvm.org/D13081, and I think it should cover this case as well. Thanks anyway for your work, and please keep contributing if you feel the check isn't perfect!
Remove remaining commented code.
I think that a more generic approach might be better, as discussed in http://reviews.llvm.org/D10933#225746. It should cover this particular case as well as other cases where class names are referenced.
The unit tests should work fine, now that D12369 has been merged.
Sep 14 2015
I don't have credentials to commit it, anybody could do it for me if the diff is ok?
Sep 2 2015
Here it is, with the typos in comments corrected, and rebased over the latest trunk.