gedare added inline comments.
================
Comment at: clang/unittests/Format/FormatTest.cpp:25504
+
+#if 0
verifyFormat("if (quitelongarg !=\n"
----------------
MyDeveloperDay wrote:
> we don't do this.
OK, I'll move that other bug up my priority list, as the behavior of this test
case involves both. That bug prevents breaking the line after the operand, I
think there's some off-by-1 error somewhere in calculating the columns for
blockindent breaks. Otherwise, this patch reveals that bug as a regression in
this test case.
================
Comment at: clang/unittests/Format/FormatTest.cpp:25505-25510
verifyFormat("if (quitelongarg !=\n"
" (alsolongarg - 1)) { // ABC is a very longgggggggggggg "
"comment\n"
" return;\n"
"}",
Style);
----------------
MyDeveloperDay wrote:
> you can't remove a test and just call it good... the original author put this
> test in for a reason I assume?
understood, although it seems that the original author may not have considered
`if` statements as legitimately to be used in BlockIndented formats, since this
test case does not block indent. So, the test case will have to change anyway,
if the `if` should be blockindented.
================
Comment at: clang/unittests/Format/FormatTest.cpp:25514
+ verifyFormat("void foo() {\n"
+ " if (quitelongname < alsolongname ||\n"
+ " anotherevenlongername <=\n"
----------------
MyDeveloperDay wrote:
> isn't the breaking of `if (` and `} else if (\n` inconsistent?
The breaking is dependent on what ends up exceeding the ColumnLimit. since ` if
(` is shorter than ` } else if (` by about 5 characters, you can get different
breaks. As far as I can tell, the behavior of `AlwaysBreak` and `BlockIndent`
will only break after the opening parens if the first "argument" doesn't fit.
From what I understand, the penalties and desirable breaks come into play then,
based on breaking around commas or boolean operators. With this patch, the
behavior here is now consistent for both `AlwaysBreak` and `BlockIndent`.
This behavior is undocumented and I do not know the intention. I would think it
is better for the two options to behave consistently, but I'm also willing to
treat them differently for backward compatibility while fixing this bug.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154755/new/
https://reviews.llvm.org/D154755
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
