Re: [PATCH] scripts/checkpatch.pl: Improve guidance for LONG_LINE

[Joe Perches]

On Wed, 2014-06-25 at 23:01 -0700, Josh Triplett wrote:
> Currently, LONG_LINE just informs the user about the line length,
> leaving them to shorten the line.  Too many users run checkpatch and
> blindly follow its recommendation by splitting long lines, which almost
> invariably results in worse code.  On rare occasions, the line-width
> limit encourages sensible refactoring of nested code into functions, but
> more frequently it just results in painfully over-wrapped code.
> 
> Improve the guidance by detecting long lines that start with 4+ tabs and
> explicitly suggesting simplification or refactoring in that case.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -2146,8 +2146,16 @@ sub process {
>  		    $line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/) &&
>  		    $length > $max_line_length)
>  		{
> -			WARN("LONG_LINE",
> -			     "line over $max_line_length characters\n" . $herecurr);
> +			if ($line =~ /^\+\t{4,}/) {
> +				WARN("LONG_LINE_DEEP_NESTING",
> +				     "line over $max_line_length characters with excessive nesting (4+ tabs)\n"
> +				     . "Consider simplifying or refactoring to eliminate excessive nesting.\n"
> +				     . $herecurr);

This is not the test you want.

This is also emitting on lines that are
merely continued like
			pr_warning(format,
				   arg1,
				   arg2,
				   arg3
				   ...
				   argN);

Better would be to test only for the lines
that increase indents (like the DEEP_INDENTATION test)

$line =~ /^\+\t{4,}(?:if|for|while|do|switch)\b/

This is the treewide distribution I get for the lines
that generally produce extra indentation:

 608432 1
 242180 2
  71260 3
  16953 4
   3688 5
    758 6
    187 7
     74 8
     23 9
     17 10
      8 11
      4 12
      1 13


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/