[PATCH] D50403: [clang-format]AlignConsecutiveAssignments
owenpan abandoned this revision. owenpan added a comment. We habe `AlignCompound` now. Herald added a comment. NOTE: Clang-Format Team Automated Review Comment Your review contains a change to clang/include/clang/Format/Format.h but does not contain an update to ClangFormatStyleOptions.rst ClangFormatStyleOptions.rst is generated via clang/docs/tools/dump_format_style.py, please run this to regenerate the .rst You can validate that the rst is valid by running. ./docs/tools/dump_format_style.py mkdir -p html /usr/bin/sphinx-build -n ./docs ./html Herald added a comment. NOTE: Clang-Format Team Automated Review Comment It looks like your clang-format review does not contain any unit tests, please try to ensure all code changes have a unit test (unless this is an `NFC` or refactoring, adding documentation etc..) Add your unit tests in `clang/unittests/Format` and you can build with `ninja FormatTests`. We recommend using the `verifyFormat(xxx)` format of unit tests rather than `EXPECT_EQ` as this will ensure you change is tolerant to random whitespace changes (see FormatTest.cpp as an example) For situations where your change is altering the TokenAnnotator.cpp which can happen if you are trying to improve the annotation phase to ensure we are correctly identifying the type of a token, please add a token annotator test in `TokenAnnotatorTest.cpp` Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50403/new/ https://reviews.llvm.org/D50403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50403: [clang-format]AlignConsecutiveAssignments
rolandschulz added a comment. Is anyone still working on this or is a new author needed? For what's it worth it, I'm in favor of the first version (right hand aligned) for different sized operators. A matching bug exists at: https://bugs.llvm.org/show_bug.cgi?id=31681 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50403/new/ https://reviews.llvm.org/D50403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50403: [clang-format]AlignConsecutiveAssignments
krasimir added inline comments. Comment at: include/clang/Format/Format.h:85 + /// matching assignment operators. This includes consecutive |=, += + /// -=, /=, *=. This will result in formattings like /// \code Please add tests for these. Also it's not clear from these examples how would a block of lines using assigments spanning different number of columns would be alined, as in: ``` aaa = 1; bb += 2; c <<= 3; ``` vs. ``` aaa = 1; bb += 2; c <<= 3; ``` I think this might deserve discussion by itself before this patch can get in (personally, I'm in favor of the first version where the right hand sides are alined). Repository: rC Clang https://reviews.llvm.org/D50403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50403: [clang-format]AlignConsecutiveAssignments
owenpan requested changes to this revision.
owenpan added inline comments.
This revision now requires changes to proceed.
Comment at: include/clang/Format/Format.h:91-93
+ /// int ddd += 12;
+ /// int ee += 22;
+ /// int f += 23;
These are invalid C++ examples.
Comment at: lib/Format/WhitespaceManager.cpp:435-451
+ std::vector assignment_tokens =
+{tok::equal, tok::pipeequal, tok::caretequal, tok::percentequal,
+ tok::ampequal, tok::plusequal, tok::minusequal, tok::starequal,
+ tok::slashequal, tok::lesslessequal, tok::greatergreaterequal};
+ for (auto assignment_token : assignment_tokens)
+ {
+AlignTokens(Style,
It would be simpler to just use `isOneOf(tok::equal, tok::pipeequal, ...)` here.
Repository:
rC Clang
https://reviews.llvm.org/D50403
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50403: [clang-format]AlignConsecutiveAssignments
kristina added subscribers: djasper, klimek, kristina. kristina added a comment. Please upload your diffs with full context (use `-U9` when doing the diff) and update them. I don't think there is a code owner for this part but @klimek and @djasper do most reviews to do with clang-format, so you may consider adding them as reviewers. Repository: rC Clang https://reviews.llvm.org/D50403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50403: [clang-format]AlignConsecutiveAssignments
PDoakORNL added a comment. So it's been a week, is there an owner for clang/lib/Format? Repository: rC Clang https://reviews.llvm.org/D50403 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
