Re: [Qemu-devel] [PATCH 2/4] CODING_STYLE, checkpatch: update line length rules

[Thomas Huth <thuth@redhat.com>]

On 07/09/15 17:23, Markus Armbruster wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
>> On 07/09/15 16:22, Paolo Bonzini wrote:
>> [...]
>>> Some other interesting data:
>>>
>>> - However, only 605 files have 5 or more lines with 77+ columns, so the
>>> odds of warnings after copy-n-paste are pretty slim.
>>
>> Apart from copy-n-pasting, there is also the problem that you can run
>> "checkpatch.pl -f" on a whole file ... it would also be ugly to suddenly
>> have (much) more warnings here.
> 
> Feature.  If you run checkpatch on a whole file, you obviously do it to
> find its ugly spots.  Lines longer than 76 characters qualify.

I sometimes also use "checkpatch.pl -f" to check my own new code that
I've added to files (when I did not create patch files yet). When it
suddenly spams me because of such new line-length warnings, that would
be quite annoying, I think.

>>> Based on the above data, I suggest having 79 columns as the limit (with
>>> the "BSD exception" unfortunately).  This still makes it possible to
>>> watch diffs on an 80-column terminal without wrapping.
>>
>> 79 sounds at least better to me than 76. So if you really want to change
>> the limit, please go for 79 instead of 76.
> 
> Have you read the thread leading up to this patch?

You mean the v1 thread of this patch? I slightly recall it ... and just
looked it up again. As far as I can see, you were the only one who
suggested such a small limit, John wanted to keep it at 80 columns and
Andreas suggested 78.

I can see your point with "typographic style suggests 60 characters",
but I think you can not really fully apply this to source code - it
reads much different that natural text, e.g. there are much more
separating special characters like parentheses and commas in between.

We could maybe mention that 76 columns limit in the coding style
document, to make people aware that they should start thinking about
breaking lines in two when they reach 76 characters in a line. But
changing checkpatch.pl to use this limit really sounds too harsh to me.
Maybe 79, yes, since that really gives to the possibility to nicely view
patches in a 80 columns terminal, too. (But if you try to view a 2-way
patch in a 80-columns window, you quite masochistic anyway, so I don't
see the point in reducing it to 78). Less than 79 would IMHO cause too
much noise with the already existing code, so I really would appreciate
if checkpatch.pl would not use less than that.

 Thomas