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

[Markus Armbruster <armbru@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Line lengths above 80 characters do exist.  They are rare, but
> they happen from time to time.  An ignored rule is worse than an
> exception to the rule, so do the latter.
>
> Based on remarks from the list, make the preferred line length
> slightly lower than 80 characters, to account for extra characters
> in unified diffs (including three-way diffs) and for email quoting.
>
> Checkpatch has some code to detect doc comments that doesn't apply
> to QEMU; the usual limits apply even for doc comments in our case.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  CODING_STYLE          | 13 ++++++++++---
>  scripts/checkpatch.pl | 21 +++++++++++++++------
>  2 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/CODING_STYLE b/CODING_STYLE
> index 3c6978f..34d5526 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -31,14 +31,21 @@ Do not leave whitespace dangling off the ends of lines.
>  
>  2. Line width
>  
> -Lines are 80 characters; not longer.
> +Lines should be 76 characters; try not to make them longer.
> +
> +Sometimes it is hard to do, especially when dealing with QEMU subsystems
> +that use long function or symbol names.  Even in that case, do not make
> +lines much longer than 76 characters.
>  
>  Rationale:
>   - Some people like to tile their 24" screens with a 6x4 matrix of 80x24
> -   xterms and use vi in all of them.  The best way to punish them is to
> -   let them keep doing it.
> +   xterms and use vi in all of them.  They also examine diffs (and three-way
> +   diffs) on an 80-column terminal, accounting for two extra characters.
> +   The best way to punish them is to let them keep doing it.
>   - Code and especially patches is much more readable if limited to a sane
>     line length.  Eighty is traditional.
> + - The four-space indentation makes the most common excuse ("But look
> +   at all that white space on the left!") moot.
>   - It is the QEMU coding style.
>  
>  3. Naming
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7f0aae9..bc32d8f 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1466,14 +1466,23 @@ sub process {
>  # check we are in a valid source file if not then ignore this hunk
>  		next if ($realfile !~ /\.(h|c|cpp|s|S|pl|sh)$/);
>  
> -#80 column limit
> -		if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ &&
> -		    $rawline !~ /^.\s*\*\s*\@$Ident\s/ &&
> +#90 column limit
> +		if ($line =~ /^\+/ &&
>  		    !($line =~ /^\+\s*$logFunctions\s*\(\s*(?:(KERN_\S+\s*|[^"]*))?"[X\t]*"\s*(?:,|\)\s*;)\s*$/ ||
> -		    $line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/) &&
> -		    $length > 80)
> +		    !($line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/))
>  		{
> -			WARN("line over 80 characters\n" . $herecurr);
> +			if ($length > 90) {
> +				ERROR("line over 90 characters\n" . $herecurr);
> +			} elsif ($length > 76 && !($rawline =~ /^\+ \* /)) {
> +				# The BSD license blurb has 80 character lines.
> +				# Avoid warning on cut-and-pasted license text.

Why not simply reflow all the offending license blurbs?

Want me to prep such a patch?

> +				WARN("line over 76 characters\n" . $herecurr);
> +			} elsif ($length > 80) {
> +				# Do not confuse the user by introducing yet
> +				# another limit (80 characters).  Technically,
> +				# this line *is* over 76 characters.
> +				WARN("line over 76 characters\n" . $herecurr);
> +			}
>  		}
>  
>  # check for spaces before a quoted newline