checkpatch: fix false positive for COMMIT_LOG_LONG_LINE with URLs
PATCH 1/1 checkpatch: fix false positive for COMMIT_LOG_LONG_LINE with URLs
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:"
--- scripts/checkpatch.pl | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
scripts/checkpatch.pl +8 -2
@@ -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; }
Signature · 6 lines
2.17.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
scripts/checkpatch.pl
Quoted 21 lines · Aditya Srivastava
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.htmlAvoid 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(-)scripts/checkpatch.pl +0
-0
[]
Quoted 4 lines
scripts/checkpatch.pl +0
-0@@ -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)) {
scripts/checkpatch.pl sub process { · line 3042
Quoted 10 lines
scripts/checkpatch.pl +8
-2- 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
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Quoted 40 lines · Joe Perches
scripts/checkpatch.pl
Quoted 21 lines · Aditya Srivastava
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.htmlAvoid 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(-)scripts/checkpatch.pl +0
-0[]
Quoted 4 lines
scripts/checkpatch.pl +0
-0@@ -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)) {
scripts/checkpatch.pl sub process { · line 3042
Quoted 10 lines
scripts/checkpatch.pl +8
-2- 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.
Quoted 6 lines
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 :)
Quoted 7 lines
Make the URI/URL check follow the styles allowed by RFC 3986. Look at the
long_linecheck around line 3500 introduced by commit2e4bbbc550be336cbb3defc67430fc0700aa1426
Author: Andreas Brauchli <[email protected]>
Date: Tue Feb 6 15:38:45 2018 -0800
checkpatch: allow long lines containing URL
Quoted 7 lines
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.
Quoted 11 lines
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
bdc48fa11e46f867ea4d75fa59ee87a7f48be144checkpatch/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
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v2 1/1 checkpatch: fix false positive for COMMIT_LOG_LONG_LINE with URLs
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:"
---
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(-)
scripts/checkpatch.pl +8 -3
@@ -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
Signature · 6 lines
2.17.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
Quoted 40 lines · Aditya Srivastava
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.htmlAvoid 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 URLscripts/checkpatch.pl | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)scripts/checkpatch.pl +1
-3@@ -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) {
scripts/checkpatch.pl sub process { · line 3045
Quoted 10 lines
scripts/checkpatch.pl +7
-0+ 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
_______________________________________________
Linux-kernel-mentees mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
v3 cover checkpatch: fix false positive for COMMIT_LOG_LONG_LINE and provide fix
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(-)
Signature · 6 lines
2.17.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 1/2 checkpatch: fix false positive for COMMIT_LOG_LONG_LINE with URLs
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
Avoid giving users warning for character limit for such cases, instead suggest them to prefix the URLs with "Link:"
--- scripts/checkpatch.pl | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
scripts/checkpatch.pl +8 -3
@@ -3023,9 +3023,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*[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
Signature · 6 lines
2.17.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees
PATCH v3 2/2 checkpatch: add fix option for COMMIT_LOG_LONG_LINE with URLs
In reply to Aditya SrivastavaCurrently 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:"
--- scripts/checkpatch.pl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
scripts/checkpatch.pl +6 -3
@@ -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);
Signature · 6 lines
2.17.1 _______________________________________________ Linux-kernel-mentees mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees