[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In https://reviews.llvm.org/D40068#931679, @Typz wrote:

> Generally, this indeed improves the situation (though I cannot say much about the code itself, it is still too subtle for my shallow knowledge of clang-format).
>
> But it seems to give some strange looking result with long comments: it seems like the decision is made at each line (e.g. is it better to wrap this line or overflow a bit), so we can get a comment where each line overflows by a few characters, even if the total is worse... For exemple, say we have a 100 lines of comment, with 9 characters overflow on each line, and an excess character penalty of 30 : with this patch nothing will be re-wrapped (9*30 = 270 is less the the 300 penalty for wrapping); but the total penatly would be 900....
>
> (btw, it seems this got merged, but the ticket does not reflect it)


Ugh, sorry, I think an arc patch I did without --create on a branch that was branched after the original patch was sent out broke phab :( :( This is not actually the state the patch was in when submitted, or the state it was LG'ed in (see https://reviews.llvm.org/rL318515 for the smaller patch)

In https://reviews.llvm.org/D40068#931679, @Typz wrote:

> Generally, this indeed improves the situation (though I cannot say much about the code itself, it is still too subtle for my shallow knowledge of clang-format).
>
> But it seems to give some strange looking result with long comments: it seems like the decision is made at each line (e.g. is it better to wrap this line or overflow a bit), so we can get a comment where each line overflows by a few characters, even if the total is worse... For exemple, say we have a 100 lines of comment, with 9 characters overflow on each line, and an excess character penalty of 30 : with this patch nothing will be re-wrapped (9*30 = 270 is less the the 300 penalty for wrapping); but the total penatly would be 900....
>
> (btw, it seems this got merged, but the ticket does not reflect it)





https://reviews.llvm.org/D40068