2024-02-03 19:55:42

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 0/2] checkpatch: relax >75 warning, adjust report format

1. current checkpatch warns: "Prefer a maximum 75 chars per line"
on [1]:

[1] https://lore.kernel.org/lkml/[email protected]

That is overly strict. Instead allow long non-whitespace strings
(\S+) to exceed 75 chars, after tolerating \s in 1st 10 chars.

This is more permissive than the current Fixes/link detector (which I
preserved), IMHO checkpatch shouldn't be that fussy; it doesn't verify
URLs either.

2. adjust report format for nicer pararaphs.

Current checkpatch format uses -------------- lines to segment the
many per-patch reports, but then inserts a blank line just above the
last per-patch report line, breaking the paragraph nature of that
report.

Move the blank line down, which places it just above the -------- line
starting the next per-patch report. Also wrap the per-patch summary
line, so its columnar position is independent of the patch-name (and
its length), and so your terminal window doesn't have to wrap the
line. This makes the report easier to visually scroll/scan, since the
status is always in the same column.

IOW, from this:

0001-checkpatch-report last line, looks part of 0002
--------------------------------------------------------------
0002-checkpatch-minor-whitespace-changes-for-readability.patch
--------------------------------------------------------------
total: 0 errors, 0 warnings, 15 lines checked

0002-checkpatch-minor-whitespace-changes-for-readability.patch has ....
-------------------------------------------------
next-patch ...

To this:

--------------------------------------------------------------
0002-checkpatch-minor-whitespace-changes-for-readability.patch
--------------------------------------------------------------
total: 0 errors, 0 warnings, 15 lines checked
0002-checkpatch-minor-whitespace-changes-for-readability.patch
has no obvious style problems and is ready for submission.

-------------------------------------------------
next-patch ...

Jim Cromie (2):
checkpatch: tolerate long lines w/o spaces
checkpatch: minor whitespace changes for readability

scripts/checkpatch.pl | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

--
2.43.0



2024-02-03 19:55:51

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 1/2] checkpatch: tolerate long lines w/o spaces

checkpatch complains about long lines (>75 chars), but allows a few
exceptions:

file delta changes
filename then :
A Fixes:, link or signature tag line

Add another exception, to allow a long "word" without whitespace. The
regex allows spaces in 1st 10 chars, so it allows the following [1],
which current checkpatch carps about.

[1] https://lore.kernel.org/lkml/[email protected]

Dummy-tag: test-generic-tag-format-with-url-length-bla-bla-jeez-dont-give-up-now-there.

Signed-off-by: Jim Cromie <[email protected]>
---
scripts/checkpatch.pl | 2 ++
1 file changed, 2 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9c4c4a61bc83..dc17c6da3af2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3271,6 +3271,8 @@ sub process {
# filename then :
$line =~ /^\s*(?:Fixes:|$link_tags_search|$signature_tags)/i ||
# A Fixes:, link or signature tag line
+ $line =~ /^.{10}\S+$/ ||
+ # a long single "word" without whitespace
$commit_log_possible_stack_dump)) {
WARN("COMMIT_LOG_LONG_LINE",
"Prefer a maximum 75 chars per line (possible unwrapped commit description?)\n" . $herecurr);
--
2.43.0


2024-02-03 19:58:13

by Jim Cromie

[permalink] [raw]
Subject: [PATCH 2/2] checkpatch: minor whitespace changes for readability

Change the per-patch report format slightly, so its more paragraphic,
and easier to peruse/scan.

OLD FORM:

0001-checkpatch-report last line, looks part of 0002 has no ...
--------------------------------------------------------------
0002-checkpatch-minor-whitespace-changes-for-readability.patch
--------------------------------------------------------------
total: 0 errors, 0 warnings, 15 lines checked

0002-checkpatch-minor-whitespace-changes-for-readability.patch has no ...
--------------------------------------------------------------

NEW FORM:

--------------------------------------------------------------
0002-checkpatch-minor-whitespace-changes-for-readability.patch
--------------------------------------------------------------
total: 0 errors, 0 warnings, 15 lines checked
0002-checkpatch-minor-whitespace-changes-for-readability.patch
has no obvious style problems and is ready for submission.

--------------------------------------------------------------

For the optimum (warning free) patchset, the new report format is a
series of single paragraphs (with the --- banners), one for each patch
in the series. This is trivial to scan/peruse, and is also more
visually distinct from warning/error reports.

Signed-off-by: Jim Cromie <[email protected]>
---
scripts/checkpatch.pl | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index dc17c6da3af2..cd249ae9abdd 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -7796,12 +7796,12 @@ EOM
}

if ($quiet == 0) {
- print "\n";
if ($clean == 1) {
- print "$vname has no obvious style problems and is ready for submission.\n";
+ print "$vname \n has no obvious style problems and is ready for submission.\n";
} else {
- print "$vname has style problems, please review.\n";
+ print "$vname \n has style problems, please review.\n";
}
+ print "\n";
}
return $clean;
}
--
2.43.0


2024-02-05 17:39:51

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 2/2] checkpatch: minor whitespace changes for readability

On Sat, 2024-02-03 at 12:55 -0700, Jim Cromie wrote:
> Change the per-patch report format slightly, so its more paragraphic,
> and easier to peruse/scan.
>
> OLD FORM:
>
> 0001-checkpatch-report last line, looks part of 0002 has no ...
> --------------------------------------------------------------
> 0002-checkpatch-minor-whitespace-changes-for-readability.patch
> --------------------------------------------------------------
> total: 0 errors, 0 warnings, 15 lines checked
>
> 0002-checkpatch-minor-whitespace-changes-for-readability.patch has no ...
> --------------------------------------------------------------
>
> NEW FORM:
>
> --------------------------------------------------------------
> 0002-checkpatch-minor-whitespace-changes-for-readability.patch
> --------------------------------------------------------------
> total: 0 errors, 0 warnings, 15 lines checked
> 0002-checkpatch-minor-whitespace-changes-for-readability.patch
> has no obvious style problems and is ready for submission.
>
> --------------------------------------------------------------
>
> For the optimum (warning free) patchset, the new report format is a
> series of single paragraphs (with the --- banners), one for each patch
> in the series. This is trivial to scan/peruse, and is also more
> visually distinct from warning/error reports.
>
> Signed-off-by: Jim Cromie <[email protected]>
> ---
> scripts/checkpatch.pl | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index dc17c6da3af2..cd249ae9abdd 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -7796,12 +7796,12 @@ EOM
> }
>
> if ($quiet == 0) {
> - print "\n";
> if ($clean == 1) {
> - print "$vname has no obvious style problems and is ready for submission.\n";
> + print "$vname \n has no obvious style problems and is ready for submission.\n";

Space after filename / $vname isn't necessary.

> } else {
> - print "$vname has style problems, please review.\n";
> + print "$vname \n has style problems, please review.\n";

here too.

> }
> + print "\n";
> }
> return $clean;
> }


2024-02-05 18:02:32

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH 1/2] checkpatch: tolerate long lines w/o spaces

On Sat, 2024-02-03 at 12:55 -0700, Jim Cromie wrote:
> checkpatch complains about long lines (>75 chars), but allows a few
> exceptions:
>
> file delta changes
> filename then :
> A Fixes:, link or signature tag line
>
> Add another exception, to allow a long "word" without whitespace. The
> regex allows spaces in 1st 10 chars, so it allows the following [1],
> which current checkpatch carps about.
>
> [1] https://lore.kernel.org/lkml/[email protected]
>
> Dummy-tag: test-generic-tag-format-with-url-length-bla-bla-jeez-dont-give-up-now-there.
>
> Signed-off-by: Jim Cromie <[email protected]>
> ---
> scripts/checkpatch.pl | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 9c4c4a61bc83..dc17c6da3af2 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3271,6 +3271,8 @@ sub process {
> # filename then :
> $line =~ /^\s*(?:Fixes:|$link_tags_search|$signature_tags)/i ||
> # A Fixes:, link or signature tag line
> + $line =~ /^.{10}\S+$/ ||
> + # a long single "word" without whitespace

Perhaps better would be
$line =! /^(?:\S+\s+)?\S+$/ ||


> $commit_log_possible_stack_dump)) {
> WARN("COMMIT_LOG_LONG_LINE",
> "Prefer a maximum 75 chars per line (possible unwrapped commit description?)\n" . $herecurr);