klimek added a comment. In https://reviews.llvm.org/D33589#931723, @Typz wrote:
> In https://reviews.llvm.org/D33589#925903, @klimek wrote: > > > I think this patch doesn't handle a couple of cases that I'd like to see > > handled. A counter-proposal with different trade-offs is in > > https://reviews.llvm.org/D40068. > > > Can you provide more info on these cases? Are they all added to the tests of > https://reviews.llvm.org/D40068? > > It may be simpler (though not to my eyes, I am not knowledgeable enough to > really understand how you go this fixed...), and works fine for "almost > correct" comments: e.g. when there are indeed just a few extra characters > overall. But it still procudes strange result when each line of the (long) > comment is too long, but not enough to trigger a line-wrap by itself. > > Since that version has landed already, not sure how to improve on this. I > could probably rewrite my patch on master, but it seems a bit redundant. As a > simpler fix, I could imagine adding a "total" overflow counter, to allow > detecting the situation; but when this is detected (e.g. on subsequent lines) > we would need to "backtrack" and revisit the initial decision... Yes, I added all tests I was thinking of to the patch. Note that we can always go back on submitted patches / do things differently. As my patch fulfilled all your tests, I (perhaps incorrectly?) assumed it solved your use cases - can you give me a test case of what you would want to happen that doesn't happen with my patch? Are you, for example, saying that in the case Limit; 13 // foo bar baz foo bar baz foo bar baz you'd not want // foo bar baz // foo bar baz // foo bar baz If the overall penalty of the protruding tokens is - what? More than 1 break penalty? If you care about more than 3x break penalty (which I would think is correct), the local algorithm will work, as local decisions will make sure the overall penalty cannot be exceeded. https://reviews.llvm.org/D33589 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
