Typz added a comment. > @klimek wrote: > In the above example, we add 3 line breaks, and we'd add 1 (or more) > additional line breaks when reflowing below the column limit. > I agree that that can lead to different overall outcomes, but I don't see > how the approach of this patch really fixes it - it will only ever reflow > below the column limit, so it'll also lead to states for long lines where > reflowing and leaving chars over the line limit might be the overall best > choice (unless I'm missing something)?
Definitely, this patch may not find the 'optimal' solution. What I mean is that we reduce the `PenaltyExcessCharacter` value to allow "occasionally" breaking the limit: instead of a hard limit, we want to allow lines to sometimes break the limit, but definitely not *all* the time. Both patch work fine when the code is "correct", i.e. there is indeed only a few lines which break the limit. But the local decision approach behaves really wrong IMHO when the code is formatted beyond the column: it can very easily reformat in such a way that the comment is reflown to what looks like a longer column limit. I currently have a ratio of 10 between PenaltyBreakComment and PenaltyExcessCharacter (which empirically seemed to give a decent compromise, and match how our code is formatted; I need to try more with your patch, to see if we can get better values...): with this setting, a "non wrapped" comment will actually be reflown to ColumnLimit+10... When we do indeed reflow, I think we may be stricter than this, to get something that really looks like it obeys the column limit. If this is 'optimal' in the sense that we may have some overflow still, that is fine, but really not the primary requirement IMHO. https://reviews.llvm.org/D33589 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
