Currently checkpatch warns for long line in commit messages even for
URL lines.
An evaluation over v4.13..v5.8 showed that out of ~11000 warnings for
this class, around 790 are due to the line containing link.
E.g. running checkpatch on commit 3cde818cd02b ("ASoC: topology:
Consolidate how dtexts and dvalues are freed") reports this warning:
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-January/144761.html
Avoid giving users warning for character limit, instead suggest them to
prefix the URLs with "Link:"
Signed-off-by: Aditya Srivastava <[email protected]>
---
scripts/checkpatch.pl | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index abd5a3d2e913..23da1f50fe6a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3032,8 +3032,14 @@ sub process {
$line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
# A Fixes: or Link: line or signature tag line
$commit_log_possible_stack_dump)) {
- WARN("COMMIT_LOG_LONG_LINE",
- "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
+ if ($line =~ /(?:http|https|ftp):\/\//) {
+ WARN("COMMIT_LOG_LONG_LINE",
+ "Consider prefixing the URL with 'Link:'\n" . $herecurr);
+ }
+ else {
+ WARN("COMMIT_LOG_LONG_LINE",
+ "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
+ }
$commit_log_long_line = 1;
}
--
2.17.1
On Thu, 2020-12-17 at 19:12 +0530, Aditya Srivastava wrote:
> Currently checkpatch warns for long line in commit messages even for
> URL lines.
>
> An evaluation over v4.13..v5.8 showed that out of ~11000 warnings for
> this class, around 790 are due to the line containing link.
>
> E.g. running checkpatch on commit 3cde818cd02b ("ASoC: topology:
> Consolidate how dtexts and dvalues are freed") reports this warning:
>
> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-January/144761.html
>
> Avoid giving users warning for character limit, instead suggest them to
> prefix the URLs with "Link:"
>
> Signed-off-by: Aditya Srivastava <[email protected]>
> ---
> ?scripts/checkpatch.pl | 10 ++++++++--
> ?1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -3032,8 +3032,14 @@ sub process {
> ? $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
> ? # A Fixes: or Link: line or signature tag line
> ? $commit_log_possible_stack_dump)) {
> - WARN("COMMIT_LOG_LONG_LINE",
> - "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
> + if ($line =~ /(?:http|https|ftp):\/\//) {
> + WARN("COMMIT_LOG_LONG_LINE",
> + "Consider prefixing the URL with 'Link:'\n" . $herecurr);
> + }
> + else {
> + WARN("COMMIT_LOG_LONG_LINE",
> + "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
> + }
NAK.
Aditya, you've submitted several patches to checkpatch and
you should know better by now what coding style is necessary
for acceptance.
} else {
Make the URI/URL check follow the styles allowed by RFC 3986.
Look at the long_line check around line 3500 introduced by
commit 2e4bbbc550be336cbb3defc67430fc0700aa1426
Author: Andreas Brauchli <[email protected]>
Date: Tue Feb 6 15:38:45 2018 -0800
checkpatch: allow long lines containing URL
Also likely the URI should not be allowed to exceed the line
maximum unless it's the first non-whitespace of the line and
not starting after some other word in the line.
Lastly, this sets $commit_log_long_line even for lines that
are now nominally exempted from the long line check.
The number of nominal fixes you showed above is not correct.
Retrospective testing of checkpatch using --git history
should be aware of changes to checkpatch.
This should count only lines from 75 to 80 chars for the
commit range you tested and only for 75 to 100 for commits
after checkpatch changed its allowed long line maximum in
commit bdc48fa11e46f867ea4d75fa59ee87a7f48be144
checkpatch/coding-style: deprecate 80-column warning
On 17/12/20 10:33 pm, Joe Perches wrote:
> On Thu, 2020-12-17 at 19:12 +0530, Aditya Srivastava wrote:
>> Currently checkpatch warns for long line in commit messages even for
>> URL lines.
>>
>> An evaluation over v4.13..v5.8 showed that out of ~11000 warnings for
>> this class, around 790 are due to the line containing link.
>>
>> E.g. running checkpatch on commit 3cde818cd02b ("ASoC: topology:
>> Consolidate how dtexts and dvalues are freed") reports this warning:
>>
>> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
>> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-January/144761.html
>>
>> Avoid giving users warning for character limit, instead suggest them to
>> prefix the URLs with "Link:"
>>
>> Signed-off-by: Aditya Srivastava <[email protected]>
>> ---
>> scripts/checkpatch.pl | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
>> @@ -3032,8 +3032,14 @@ sub process {
>> $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
>> # A Fixes: or Link: line or signature tag line
>> $commit_log_possible_stack_dump)) {
>> - WARN("COMMIT_LOG_LONG_LINE",
>> - "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
>> + if ($line =~ /(?:http|https|ftp):\/\//) {
>> + WARN("COMMIT_LOG_LONG_LINE",
>> + "Consider prefixing the URL with 'Link:'\n" . $herecurr);
>> + }
>> + else {
>> + WARN("COMMIT_LOG_LONG_LINE",
>> + "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
>> + }
>
> NAK.
>
> Aditya, you've submitted several patches to checkpatch and
> you should know better by now what coding style is necessary
> for acceptance.
>
> } else {
>
Sorry, I missed it. Will change it :)
> Make the URI/URL check follow the styles allowed by RFC 3986.
> Look at the long_line check around line 3500 introduced by
> commit 2e4bbbc550be336cbb3defc67430fc0700aa1426
> Author: Andreas Brauchli <[email protected]>
> Date: Tue Feb 6 15:38:45 2018 -0800
> checkpatch: allow long lines containing URL
>
> Also likely the URI should not be allowed to exceed the line
> maximum unless it's the first non-whitespace of the line and
> not starting after some other word in the line.
>
> Lastly, this sets $commit_log_long_line even for lines that
> are now nominally exempted from the long line check.
>
Okay. I'll make these changes and send the modified patch.
> The number of nominal fixes you showed above is not correct.
>
> Retrospective testing of checkpatch using --git history
> should be aware of changes to checkpatch.
>
> This should count only lines from 75 to 80 chars for the
> commit range you tested and only for 75 to 100 for commits
> after checkpatch changed its allowed long line maximum in
> commit bdc48fa11e46f867ea4d75fa59ee87a7f48be144
> checkpatch/coding-style: deprecate 80-column warning
>
Joe, I think this is probably true only for "WARNING:LONG_LINE".
However, here I have analyzed the count for
"WARNING:COMMIT_LOG_LONG_LINE".
I ran git log on these lines. Probably these lines were last changed
at 2a076f40d8c9be95bee7bcf18436655e1140447f.
I think I can change the count with exact numbers.
What do you think?
Thanks
Aditya
Currently checkpatch warns for long line in commit messages even for
URL lines.
An evaluation over v4.13..v5.8 showed that out of 11729 warnings for
this class, around 299 are due to line starting with URL.
E.g., running checkpatch on commit 3cde818cd02b ("ASoC: topology:
Consolidate how dtexts and dvalues are freed") reports this warning:
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-January/144761.html
Avoid giving users warning for character limit for such cases, instead
suggest them to prefix the URLs with "Link:"
Signed-off-by: Aditya Srivastava <[email protected]>
---
changes in v2:
- Fix coding style ('} else {')
- Make the URL check follow RFC 3986 style
- Give warning only if the URL is first non-whitespace of the line
- Set $commit_log_long_line only for else case
- Fix the warning count with exact figures and according to first non-space char as URL
scripts/checkpatch.pl | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index abd5a3d2e913..bf77bd0b22cf 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3032,9 +3032,14 @@ sub process {
$line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
# A Fixes: or Link: line or signature tag line
$commit_log_possible_stack_dump)) {
- WARN("COMMIT_LOG_LONG_LINE",
- "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
- $commit_log_long_line = 1;
+ if ($line =~ /^\s*\b[a-z][\w\.\+\-]*:\/\/\S+/i) {
+ WARN("COMMIT_LOG_LONG_LINE",
+ "Consider prefixing the URL with 'Link:'\n" . $herecurr);
+ } else {
+ WARN("COMMIT_LOG_LONG_LINE",
+ "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
+ $commit_log_long_line = 1;
+ }
}
# Reset possible stack dump if a blank line is found
--
2.17.1
On 18/12/20 5:41 pm, Aditya Srivastava wrote:
> Currently checkpatch warns for long line in commit messages even for
> URL lines.
>
> An evaluation over v4.13..v5.8 showed that out of 11729 warnings for
> this class, around 299 are due to line starting with URL.
>
> E.g., running checkpatch on commit 3cde818cd02b ("ASoC: topology:
> Consolidate how dtexts and dvalues are freed") reports this warning:
>
> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-January/144761.html
>
> Avoid giving users warning for character limit for such cases, instead
> suggest them to prefix the URLs with "Link:"
>
> Signed-off-by: Aditya Srivastava <[email protected]>
> ---
> changes in v2:
> - Fix coding style ('} else {')
> - Make the URL check follow RFC 3986 style
> - Give warning only if the URL is first non-whitespace of the line
> - Set $commit_log_long_line only for else case
> - Fix the warning count with exact figures and according to first non-space char as URL
>
> scripts/checkpatch.pl | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index abd5a3d2e913..bf77bd0b22cf 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3032,9 +3032,14 @@ sub process {
> $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
> # A Fixes: or Link: line or signature tag line
> $commit_log_possible_stack_dump)) {
> - WARN("COMMIT_LOG_LONG_LINE",
> - "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
> - $commit_log_long_line = 1;
> + if ($line =~ /^\s*\b[a-z][\w\.\+\-]*:\/\/\S+/i) {
> + WARN("COMMIT_LOG_LONG_LINE",
> + "Consider prefixing the URL with 'Link:'\n" . $herecurr);
> + } else {
> + WARN("COMMIT_LOG_LONG_LINE",
> + "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
> + $commit_log_long_line = 1;
> + }
> }
>
> # Reset possible stack dump if a blank line is found
>
Hi Joe
You probably missed this patch. Please review :)
Thanks
Aditya
Currently, checkpatch gives COMMIT_LOG_LONG_LINE warning even for URL
lines, which should be avoided.
An evaluation over v5.6..v5.8 found that out of 1703 warnings reported
by this class, 161 are due to the line containg URLs. Out of these 161,
53 are due to lines where URL is the first non-whitespace character of
the line.
Fix this false positive by suggesting to prefix the URL with "Link:".
Also provide the fix option to the user.
* Applies perfectly on next-20210108
Changes in v2:
- Fix coding style ('} else {')
- Make the URL check follow RFC 3986 style
- Give warning only if the URL is first non-whitespace of the line
- Set $commit_log_long_line only for else case
- Fix the warning count with exact figures and according to first non-space char as URL
Changes in v3:
- Provide fix option for the warning
- Update the warning count with v5.6..v5.8
- Update regex with /^\s*[a-z][\w\.\+\-]*:\/\/\S+/i (earlier: /^\s*\b[a-z][\w\.\+\-]*:\/\/\S+/i)
Aditya Srivastava (2):
checkpatch: fix false positive for COMMIT_LOG_LONG_LINE with URLs
checkpatch: add fix option for COMMIT_LOG_LONG_LINE with URLs
scripts/checkpatch.pl | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
--
2.17.1
Currently checkpatch warns for long line in commit messages even for
URL lines.
An evaluation over v5.6..v5.8 found that out of 1703 warnings reported
by this class, 161 are due to the line containg URLs. Out of these 161,
53 are due to lines where URL is the first non-whitespace character of
the line.
E.g. running checkpatch on commit 3cde818cd02b ("ASoC: topology:
Consolidate how dtexts and dvalues are freed") reports this warning:
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-January/144761.html
Provide a simple fix option by prefixing the first non-whitespace
character of the line with "Link:"
Signed-off-by: Aditya Srivastava <[email protected]>
---
scripts/checkpatch.pl | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e8851ce73149..7030c4d6d126 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3023,9 +3023,12 @@ sub process {
$line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i ||
# A Fixes: or Link: line or signature tag line
$commit_log_possible_stack_dump)) {
- if ($line =~ /^\s*[a-z][\w\.\+\-]*:\/\/\S+/i) {
- WARN("COMMIT_LOG_LONG_LINE",
- "Consider prefixing the URL with 'Link:'\n" . $herecurr);
+ if ($line =~ /^\s*([a-z][\w\.\+\-]*:\/\/\S+)/i) {
+ if (WARN("COMMIT_LOG_LONG_LINE",
+ "Consider prefixing the URL with 'Link:'\n" . $herecurr) &&
+ $fix) {
+ $fixed[$fixlinenr] = "Link: $1";
+ }
} else {
WARN("COMMIT_LOG_LONG_LINE",
"Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr);
--
2.17.1