From: Josh Triplett <josh@joshtriplett.org>
To: Joe Perches <joe@perches.com>, Andy Whitcroft <apw@canonical.com>,
gregkh@linux-foundation.org, akpm@linux-foundation.org
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH] scripts/checkpatch.pl: Improve guidance for LONG_LINE
Date: Wed, 25 Jun 2014 23:01:42 -0700 [thread overview]
Message-ID: <20140626060136.GA2528@thin> (raw)
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.
This does not introduce or remove any warnings; it just changes the text
of a warning that checkpatch would already emit, to emphasize a
preference for simplifying or refactoring complex, deeply nested code,
rather than just wrapping it but leaving it complex.
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
Of the ~267000 80+ character lines in the current kernel source, this
new guidance would trigger on ~67000 of them.
Other things I tested, but did not include in this patch, to minimize
controversy or subjectivity: flagging long lines with 2 or more logical
operations (&& or ||) to suggest wrapping at the operators, and flagging
long lines with 30+ character identifiers to suggest consideration of
whether the identifier could remain self-documenting if shortened.
scripts/checkpatch.pl | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 010b18e..6e82f19 100755
--- 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);
+ } else {
+ WARN("LONG_LINE",
+ "line over $max_line_length characters\n"
+ . $herecurr);
+ }
}
# Check for user-visible strings broken across lines, which breaks the ability
--
2.0.0
next reply other threads:[~2014-06-26 6:01 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-26 6:01 Josh Triplett [this message]
2014-06-26 10:09 ` [PATCH] scripts/checkpatch.pl: Improve guidance for LONG_LINE Joe Perches
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140626060136.GA2528@thin \
--to=josh@joshtriplett.org \
--cc=akpm@linux-foundation.org \
--cc=apw@canonical.com \
--cc=gregkh@linux-foundation.org \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox