All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Lucas Mateus Castro(alqotel)" <lucas.araujo@eldorado.org.br>
To: qemu-devel@nongnu.org
Cc: "Lucas Mateus Castro(alqotel)" <lucas.araujo@eldorado.org.br>,
	"Peter Maydell" <peter.maydell@linaro.org>,
	"Markus Armbruster" <armbru@redhat.com>,
	"Philippe Mathieu-Daudé" <philippe.mathieu.daude@gmail.com>,
	"Gan Qixin" <ganqixin@huawei.com>,
	"Daniel Henrique Barboza" <danielhb413@gmail.com>
Subject: [RFC PATCH RESEND] scripts/checkpatch.pl: Change line limit warning
Date: Mon,  6 Jun 2022 11:34:18 -0300	[thread overview]
Message-ID: <20220606143419.656598-1-lucas.araujo@eldorado.org.br> (raw)

The QEMU documentation mentions that lines should be up to 80
characters and that the script checkpatch will warn at 100 characters,
but the script warns at 80 characters and throw and error at 90, so
this commit changes to warn at 100.

As to why extend, the argument that resulted in the change of the
docs was that trying to always wrap to 80 columns could result in less
readable code, so sometimes not wrapping was the better choice and in
those circumstances checkpatch could nudge people into creating less
readable code.

A 132 error limit is put to catch overly big lines.

Based-on: 20201105154208.12442-1-ganqixin@huawei.com
Signed-off-by: Lucas Mateus Castro(alqotel) <lucas.araujo@eldorado.org.br>
---
Currently there's a disagreement between the checkpatch code and the
documentation, this RFC just changes the checkpatch to match the
documentation.
But there was a discussion in 2020 as the best way to deal with this,
some alternatives mentioned are: change the warning to remind people to
not blindly wrap just because of the warning, change to warn at 90 columns
(which would mean changing the column limit for the error as well). If any
of those are preferred I'll send a next version updating the documentation
as well as changing checkpatch.pl to the preferred behavior.
---
 scripts/checkpatch.pl | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d900d18048..2c2d7b31ab 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1639,12 +1639,12 @@ sub process {
 		if ($line =~ /^\+/ &&
 		    !($line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/) &&
 		    !($rawline =~ /^[^[:alnum:]]*https?:\S*$/) &&
-		    $length > 80)
+		    $length > 100)
 		{
-			if ($length > 90) {
-				ERROR("line over 90 characters\n" . $herecurr);
+			if ($length > 132) {
+				ERROR("line over 132 characters\n" . $herecurr);
 			} else {
-				WARN("line over 80 characters\n" . $herecurr);
+				WARN("line over 100 characters\n" . $herecurr);
 			}
 		}
 
@@ -1838,13 +1838,8 @@ sub process {
 			#print "realcnt<$realcnt> ctx_cnt<$ctx_cnt>\n";
 			#print "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n";
 
-			# The length of the "previous line" is checked against 80 because it
-			# includes the + at the beginning of the line (if the actual line has
-			# 79 or 80 characters, it is no longer possible to add a space and an
-			# opening brace there)
 			if ($#ctx == 0 && $ctx !~ /{\s*/ &&
-			    defined($lines[$ctx_ln - 1]) && $lines[$ctx_ln - 1] =~ /^\+\s*\{/ &&
-			    defined($lines[$ctx_ln - 2]) && length($lines[$ctx_ln - 2]) < 80) {
+			    defined($lines[$ctx_ln - 1]) && $lines[$ctx_ln - 1] =~ /^\+\s*\{/) {
 				ERROR("that open brace { should be on the previous line\n" .
 					"$here\n$ctx\n$rawlines[$ctx_ln - 1]\n");
 			}
-- 
2.25.1



             reply	other threads:[~2022-06-06 14:37 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-06 14:34 Lucas Mateus Castro(alqotel) [this message]
2022-06-09 13:56 ` [RFC PATCH RESEND] scripts/checkpatch.pl: Change line limit warning Peter Maydell
2022-06-21 17:19   ` Lucas Mateus Martins Araujo e Castro

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=20220606143419.656598-1-lucas.araujo@eldorado.org.br \
    --to=lucas.araujo@eldorado.org.br \
    --cc=armbru@redhat.com \
    --cc=danielhb413@gmail.com \
    --cc=ganqixin@huawei.com \
    --cc=peter.maydell@linaro.org \
    --cc=philippe.mathieu.daude@gmail.com \
    --cc=qemu-devel@nongnu.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.