curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed.
Thanks for working on this! Looks pretty good already. ================ Comment at: clang/include/clang/Format/Format.h:151 + /// When aligning assignments, whether compound assignments like + /// ``+=``'s are aligned along with ``=``'s. ---------------- You need to update the RST files when updating the doc comments here. Please use https://github.com/llvm/llvm-project/blob/main/clang/docs/tools/dump_format_style.py for that. ================ Comment at: clang/include/clang/Format/Format.h:157 + /// a &= 2; + /// bbb = 2; + /// ---------------- I guess it would be complicated to avoid adding an additional space here. I mean, it could be: ``` a &= 2; bbb = 2; ``` And with 3-char operators, there's one more space. ================ Comment at: clang/unittests/Format/FormatTest.cpp:16399 + Alignment.AlignCompoundAssignments = true; + verifyFormat("aa <= 5;\n" + "a &= 5;\n" ---------------- Please test spacing in separate `verifyFormat` cases as in the Format.h option description. ================ Comment at: clang/unittests/Format/FormatTest.cpp:16408 + "dvsdsv |= 5;\n" + "int dsvvdvsdvvv = 123;", + Alignment); ---------------- Please test shift operators `<<=` and `>>=`. ================ Comment at: clang/unittests/Format/FormatTest.cpp:16872 + // that things still get aligned. + Alignment.ColumnLimit = 20; EXPECT_EQ("int a(int x,\n" ---------------- Is it something that can be done/fixed separately? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119599/new/ https://reviews.llvm.org/D119599 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
