All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] checkpatch: allow URL >80 chars
@ 2017-11-20 12:40 Andreas Brauchli
  2017-11-21  6:02 ` Joe Perches
  0 siblings, 1 reply; 3+ messages in thread
From: Andreas Brauchli @ 2017-11-20 12:40 UTC (permalink / raw
  To: Joe Perches, Andy Whitcroft; +Cc: linux-kernel

Allow URL to exceed the 80 char limit for improved interaction in
adaption to ongoing but undocumented practice.

$ git grep -E '://\S{77}.*' -- '*.[ch]'

The patch checks that the URL is indeed on its own line in that it
allows a maximal prefix of 3 characters to account for a URL after a
comment (e.g. '// https://...')

As per RFC3986 [1], the URL format allows for alphanum, +, - and .
characters in the scheme before the separator :// as long as it starts
with a letter (e.g. https, git, f.-+).

Recognition of URIs without more context information is prone to false
positives and thus currently left out of the heuristics.

$rawline is used in the check as comments are removed from $line.

[1] https://tools.ietf.org/html/rfc3986#section-3.1

Signed-off-by: Andreas Brauchli <andreas.brauchli@sensirion.com>
---
 scripts/checkpatch.pl | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8b80bac055e4..732d87917f15 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2904,6 +2904,11 @@ sub process {
 			} elsif ($line =~ /^\+.*\bEFI_GUID\s*\(/) {
 				$msg_type = "";
 
+			# URL (w/ minimal padding e.g. "// ")
+			} elsif ($rawline =~ /^\+.*?\b([a-z][\w\.\+\-]*:\/\/\S+[^\s\)\]\.">;,]).*$/i &&
+				 length($rawline) - length($1) <= 4) {
+				$msg_type = "";
+
 			# Otherwise set the alternate message types
 
 			# a comment starts before $max_line_length
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] checkpatch: allow URL >80 chars
  2017-11-20 12:40 [PATCH v2] checkpatch: allow URL >80 chars Andreas Brauchli
@ 2017-11-21  6:02 ` Joe Perches
  2017-11-21  7:56   ` Andreas Brauchli
  0 siblings, 1 reply; 3+ messages in thread
From: Joe Perches @ 2017-11-21  6:02 UTC (permalink / raw
  To: Andreas Brauchli, Andy Whitcroft; +Cc: linux-kernel

On Mon, 2017-11-20 at 13:40 +0100, Andreas Brauchli wrote:
> Allow URL to exceed the 80 char limit for improved interaction in
> adaption to ongoing but undocumented practice.
> 
> $ git grep -E '://\S{77}.*' -- '*.[ch]'
> 
> The patch checks that the URL is indeed on its own line in that it
> allows a maximal prefix of 3 characters to account for a URL after a
> comment (e.g. '// https://...')

I think this part is not necessary or that useful.

For instance, this style

	/* some long url */

is pretty common.

> As per RFC3986 [1], the URL format allows for alphanum, +, - and .
> characters in the scheme before the separator :// as long as it starts
> with a letter (e.g. https, git, f.-+).

That seems right, thanks.

> Recognition of URIs without more context information is prone to false
> positives and thus currently left out of the heuristics.

OK.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] checkpatch: allow URL >80 chars
  2017-11-21  6:02 ` Joe Perches
@ 2017-11-21  7:56   ` Andreas Brauchli
  0 siblings, 0 replies; 3+ messages in thread
From: Andreas Brauchli @ 2017-11-21  7:56 UTC (permalink / raw
  To: Joe Perches, Andy Whitcroft; +Cc: linux-kernel

On Mon, 2017-11-20 at 22:02 -0800, Joe Perches wrote:
> On Mon, 2017-11-20 at 13:40 +0100, Andreas Brauchli wrote:
> > Allow URL to exceed the 80 char limit for improved interaction in
> > adaption to ongoing but undocumented practice.
> > 
> > $ git grep -E '://\S{77}.*' -- '*.[ch]'
> > 
> > The patch checks that the URL is indeed on its own line in that it
> > allows a maximal prefix of 3 characters to account for a URL after
> > a
> > comment (e.g. '// https://...')
> 
> I think this part is not necessary or that useful.
If it's dropped the line might not be minimal anymore. I.e. there may
be content on the same line that could easily be wrapped.

> For instance, this style
> 
> 	/* some long url */
> 
> is pretty common.
I can change the heuristics to not accept word characters outside of
the URL match. That would allow whitespace, quotes, braces and comment
characters, and thus also the example above.

> > As per RFC3986 [1], the URL format allows for alphanum, +, - and .
> > characters in the scheme before the separator :// as long as it
> > starts
> > with a letter (e.g. https, git, f.-+).
> 
> That seems right, thanks.
> 
> > Recognition of URIs without more context information is prone to
> > false
> > positives and thus currently left out of the heuristics.
> 
> OK.

Thanks for the reviews,
Andreas

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-11-21  7:56 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-20 12:40 [PATCH v2] checkpatch: allow URL >80 chars Andreas Brauchli
2017-11-21  6:02 ` Joe Perches
2017-11-21  7:56   ` Andreas Brauchli

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.