[PATCH RFC 0/3] checkpatch: add verbose mode

[PATCH RFC 0/3] checkpatch: add verbose mode

[Dwaipayan Ray]

This patch series adds a verbose mode to checkpatch.
The verbose test descriptions are loaded from the
checkpatch documentation and parsed by checkpatch.

The documentation itself needs more work. More input
on the usage and test descriptions are needed.

Also the visual aspects of the verbose mode needs
some work put into. The usual printing of verbose
descriptions can cause confusion and maybe hard to
distinguish from the actual warnings.

Please note that this is only an initial attempt
and any comments are welcome.

Dwaipayan Ray (3):
  checkpatch: add verbose mode
  docs: add documentation for checkpatch
  docs: add documentation for checkpatch

 Documentation/dev-tools/checkpatch.rst | 283 +++++++++++++++++++++++++
 Documentation/dev-tools/index.rst      |   1 +
 scripts/checkpatch.pl                  |  55 +++++
 3 files changed, 339 insertions(+)
 create mode 100644 Documentation/dev-tools/checkpatch.rst

-- 
2.30.0

[Linux-kernel-mentees] [PATCH RFC 0/3] checkpatch: add verbose mode

[Dwaipayan Ray]

This patch series adds a verbose mode to checkpatch.
The verbose test descriptions are loaded from the
checkpatch documentation and parsed by checkpatch.

The documentation itself needs more work. More input
on the usage and test descriptions are needed.

Also the visual aspects of the verbose mode needs
some work put into. The usual printing of verbose
descriptions can cause confusion and maybe hard to
distinguish from the actual warnings.

Please note that this is only an initial attempt
and any comments are welcome.

Dwaipayan Ray (3):
  checkpatch: add verbose mode
  docs: add documentation for checkpatch
  docs: add documentation for checkpatch

 Documentation/dev-tools/checkpatch.rst | 283 +++++++++++++++++++++++++
 Documentation/dev-tools/index.rst      |   1 +
 scripts/checkpatch.pl                  |  55 +++++
 3 files changed, 339 insertions(+)
 create mode 100644 Documentation/dev-tools/checkpatch.rst

-- 
2.30.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[PATCH RFC 1/3] checkpatch: add verbose mode

[Dwaipayan Ray]

Add a new verbose mode to checkpatch.pl to emit additional verbose
test descriptions.

The verbose mode is optional and can be enabled by the flag
--verbose.

The test descriptions are itself loaded from the checkpatch
documentation file at Documentation/dev-tools/checkpatch.rst.
The descriptions in the documentation are in a specified format
enclosed within .. CHECKPATCH_START and .. CHECKPATCH_END labels.

This serves a dual purpose as an external documentation to checkpatch
as well as enables flawless integration of the verbose mode.

A subtle example of the format is as follows:

Documentation/dev-tools/checkpatch.rst:

.. CHECKPATCH_START
TYPE_1

  Description line 1
  Description line 2,
  ...

TYPE_2

  Description line 1,
  etc.

.. CHECKPATCH_END

This file is parsed by checkpatch once the --verbose option
is specified. Verbose descriptions are not output when
the --terse option is specified.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 scripts/checkpatch.pl | 55 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7a323ca8a177..48eeb01074d5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -23,6 +23,8 @@ my $V = '0.32';
 use Getopt::Long qw(:config no_auto_abbrev);
 
 my $quiet = 0;
+my $verbose = 0;
+my %verbose_messages = ();
 my $tree = 1;
 my $chk_signoff = 1;
 my $chk_patch = 1;
@@ -61,6 +63,7 @@ my $spelling_file = "$D/spelling.txt";
 my $codespell = 0;
 my $codespellfile = "/usr/share/codespell/dictionary.txt";
 my $conststructsfile = "$D/const_structs.checkpatch";
+my $docsfile = "$D/../Documentation/dev-tools/checkpatch.rst";
 my $typedefsfile;
 my $color = "auto";
 my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE
@@ -78,6 +81,7 @@ Version: $V
 
 Options:
   -q, --quiet                quiet
+  -v, --verbose              verbose mode
   --no-tree                  run without a kernel tree
   --no-signoff               do not check for 'Signed-off-by' line
   --patch                    treat FILE as patchfile (default)
@@ -198,6 +202,49 @@ if (-f $conf) {
 	unshift(@ARGV, @conf_args) if @conf_args;
 }
 
+sub load_docs {
+	open(my $docs, '<', "$docsfile")
+	    or warn "$P: Can't read the documentation file $docsfile $!\n";
+
+	my @lines = ();
+	my $incl = 0;
+	while (<$docs>) {
+		my $line = $_;
+
+		if ($line =~ /\Q.. CHECKPATCH_START\E/) {
+			$incl++;
+			next;
+		} elsif ($line =~ /\Q.. CHECKPATCH_END\E/) {
+			$incl--;
+			next;
+		}
+		next if ($incl < 1);
+
+		$line =~ s/\s*\n?$//g;
+		push (@lines, $line);
+	}
+	close($docs);
+
+	my $linenr = 0;
+	my $cnt = scalar @lines;
+	while ($linenr < $cnt) {
+		while ($linenr < $cnt && $lines[$linenr] !~ /^[^\s]+/) {
+			$linenr++;
+		}
+		last if ($linenr >= $cnt);
+
+		my $message = '';
+		my $type = $lines[$linenr++];
+
+		while ($linenr < $cnt && $lines[$linenr] !~ /^[^\s]+/) {
+			$message .= trim($lines[$linenr++]) . "\n";
+		}
+
+		$message = trim($message);
+		$verbose_messages{$type} = $message;
+	}
+}
+
 # Perl's Getopt::Long allows options to take optional arguments after a space.
 # Prevent --color by itself from consuming other arguments
 foreach (@ARGV) {
@@ -208,6 +255,7 @@ foreach (@ARGV) {
 
 GetOptions(
 	'q|quiet+'	=> \$quiet,
+	'v|verbose!'	=> \$verbose,
 	'tree!'		=> \$tree,
 	'signoff!'	=> \$chk_signoff,
 	'patch!'	=> \$chk_patch,
@@ -249,6 +297,8 @@ help(0) if ($help);
 
 list_types(0) if ($list_types);
 
+load_docs() if ($verbose && !$terse);
+
 $fix = 1 if ($fix_inplace);
 $check_orig = $check;
 
@@ -2185,6 +2235,11 @@ sub report {
 		splice(@lines, 1, 1);
 		$output = join("\n", @lines);
 	}
+
+	if ($verbose && !$terse &&
+	    exists $verbose_messages{$type}) {
+		$output .= $verbose_messages{$type} . "\n\n";
+	}
 	$output = (split('\n', $output))[0] . "\n" if ($terse);
 
 	push(our @report, $output);
-- 
2.30.0

[Linux-kernel-mentees] [PATCH RFC 1/3] checkpatch: add verbose mode

[Dwaipayan Ray]

Add a new verbose mode to checkpatch.pl to emit additional verbose
test descriptions.

The verbose mode is optional and can be enabled by the flag
--verbose.

The test descriptions are itself loaded from the checkpatch
documentation file at Documentation/dev-tools/checkpatch.rst.
The descriptions in the documentation are in a specified format
enclosed within .. CHECKPATCH_START and .. CHECKPATCH_END labels.

This serves a dual purpose as an external documentation to checkpatch
as well as enables flawless integration of the verbose mode.

A subtle example of the format is as follows:

Documentation/dev-tools/checkpatch.rst:

.. CHECKPATCH_START
TYPE_1

  Description line 1
  Description line 2,
  ...

TYPE_2

  Description line 1,
  etc.

.. CHECKPATCH_END

This file is parsed by checkpatch once the --verbose option
is specified. Verbose descriptions are not output when
the --terse option is specified.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 scripts/checkpatch.pl | 55 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7a323ca8a177..48eeb01074d5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -23,6 +23,8 @@ my $V = '0.32';
 use Getopt::Long qw(:config no_auto_abbrev);
 
 my $quiet = 0;
+my $verbose = 0;
+my %verbose_messages = ();
 my $tree = 1;
 my $chk_signoff = 1;
 my $chk_patch = 1;
@@ -61,6 +63,7 @@ my $spelling_file = "$D/spelling.txt";
 my $codespell = 0;
 my $codespellfile = "/usr/share/codespell/dictionary.txt";
 my $conststructsfile = "$D/const_structs.checkpatch";
+my $docsfile = "$D/../Documentation/dev-tools/checkpatch.rst";
 my $typedefsfile;
 my $color = "auto";
 my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE
@@ -78,6 +81,7 @@ Version: $V
 
 Options:
   -q, --quiet                quiet
+  -v, --verbose              verbose mode
   --no-tree                  run without a kernel tree
   --no-signoff               do not check for 'Signed-off-by' line
   --patch                    treat FILE as patchfile (default)
@@ -198,6 +202,49 @@ if (-f $conf) {
 	unshift(@ARGV, @conf_args) if @conf_args;
 }
 
+sub load_docs {
+	open(my $docs, '<', "$docsfile")
+	    or warn "$P: Can't read the documentation file $docsfile $!\n";
+
+	my @lines = ();
+	my $incl = 0;
+	while (<$docs>) {
+		my $line = $_;
+
+		if ($line =~ /\Q.. CHECKPATCH_START\E/) {
+			$incl++;
+			next;
+		} elsif ($line =~ /\Q.. CHECKPATCH_END\E/) {
+			$incl--;
+			next;
+		}
+		next if ($incl < 1);
+
+		$line =~ s/\s*\n?$//g;
+		push (@lines, $line);
+	}
+	close($docs);
+
+	my $linenr = 0;
+	my $cnt = scalar @lines;
+	while ($linenr < $cnt) {
+		while ($linenr < $cnt && $lines[$linenr] !~ /^[^\s]+/) {
+			$linenr++;
+		}
+		last if ($linenr >= $cnt);
+
+		my $message = '';
+		my $type = $lines[$linenr++];
+
+		while ($linenr < $cnt && $lines[$linenr] !~ /^[^\s]+/) {
+			$message .= trim($lines[$linenr++]) . "\n";
+		}
+
+		$message = trim($message);
+		$verbose_messages{$type} = $message;
+	}
+}
+
 # Perl's Getopt::Long allows options to take optional arguments after a space.
 # Prevent --color by itself from consuming other arguments
 foreach (@ARGV) {
@@ -208,6 +255,7 @@ foreach (@ARGV) {
 
 GetOptions(
 	'q|quiet+'	=> \$quiet,
+	'v|verbose!'	=> \$verbose,
 	'tree!'		=> \$tree,
 	'signoff!'	=> \$chk_signoff,
 	'patch!'	=> \$chk_patch,
@@ -249,6 +297,8 @@ help(0) if ($help);
 
 list_types(0) if ($list_types);
 
+load_docs() if ($verbose && !$terse);
+
 $fix = 1 if ($fix_inplace);
 $check_orig = $check;
 
@@ -2185,6 +2235,11 @@ sub report {
 		splice(@lines, 1, 1);
 		$output = join("\n", @lines);
 	}
+
+	if ($verbose && !$terse &&
+	    exists $verbose_messages{$type}) {
+		$output .= $verbose_messages{$type} . "\n\n";
+	}
 	$output = (split('\n', $output))[0] . "\n" if ($terse);
 
 	push(our @report, $output);
-- 
2.30.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH RFC 1/3] checkpatch: add verbose mode

[Joe Perches]

On Wed, 2021-01-27 at 00:05 +0530, Dwaipayan Ray wrote:
> Add a new verbose mode to checkpatch.pl to emit additional verbose
> test descriptions.
> 
> The verbose mode is optional and can be enabled by the flag
> --verbose.
> 
> The test descriptions are itself loaded from the checkpatch

descriptions are themselves, but themselves is unnecessary.

The verbose descriptions are read from Documentation/dev-tools/checkpatch.rst

> documentation file at Documentation/dev-tools/checkpatch.rst.
> The descriptions in the documentation are in a specified format
> enclosed within .. CHECKPATCH_START and .. CHECKPATCH_END labels.
> 
> This serves a dual purpose as an external documentation to checkpatch
> as well as enables flawless integration of the verbose mode.

Using 'flawless' when describing code or documentation generally isn't true.

> A subtle example of the format is as follows:

What is subtle about an example?

If there is something subtle about an example, there's also something
wrong with the example.

> Documentation/dev-tools/checkpatch.rst:
> 
> .. CHECKPATCH_START

Nak on the keyword uses.

This should really just parse the input file whenever TYPE is found
via some fixed format and save the verbose description after that.

Use .rst Field Lists instead, and ideally, keep the list in alphabetic
order or group by similar use.

https://docutils.sourceforge.io/docs/user/rst/quickref.html#field-lists

e.g.:

:LINE_SPACING:
	Vertical space is wasted given the limited number of lines an
	editor window can display when multiple blank lines are used.

:SPACING:
	Whitespace style used in the kernel sources is described in
	ref:`Documentation/process/Coding-Style.rst section 3.1.

:TRAILING_WHITESPACE:
	Trailing whitespace should always be removed.
	Some editors highlight the trailing whitespace and cause visual
	distractions when editing files.

etc...

> @@ -2185,6 +2235,11 @@ sub report {
>  		splice(@lines, 1, 1);
>  		$output = join("\n", @lines);
>  	}
> +
> +	if ($verbose && !$terse &&
> +	    exists $verbose_messages{$type}) {
> +		$output .= $verbose_messages{$type} . "\n\n";
> +	}
>  	$output = (split('\n', $output))[0] . "\n" if ($terse);

Don't use unnecessary multiple tests of the same object, just reorder
the code instead.  And also please use c-style function parentheses
rather than bare tests.

	if ($terse) {
		$output = ...
	} elsif ($verbose && exists($verbose_messages{$type})) {
		$output .= ...
	}

Re: [Linux-kernel-mentees] [PATCH RFC 1/3] checkpatch: add verbose mode

[Joe Perches]

On Wed, 2021-01-27 at 00:05 +0530, Dwaipayan Ray wrote:
> Add a new verbose mode to checkpatch.pl to emit additional verbose
> test descriptions.
> 
> The verbose mode is optional and can be enabled by the flag
> --verbose.
> 
> The test descriptions are itself loaded from the checkpatch

descriptions are themselves, but themselves is unnecessary.

The verbose descriptions are read from Documentation/dev-tools/checkpatch.rst

> documentation file at Documentation/dev-tools/checkpatch.rst.
> The descriptions in the documentation are in a specified format
> enclosed within .. CHECKPATCH_START and .. CHECKPATCH_END labels.
> 
> This serves a dual purpose as an external documentation to checkpatch
> as well as enables flawless integration of the verbose mode.

Using 'flawless' when describing code or documentation generally isn't true.

> A subtle example of the format is as follows:

What is subtle about an example?

If there is something subtle about an example, there's also something
wrong with the example.

> Documentation/dev-tools/checkpatch.rst:
> 
> .. CHECKPATCH_START

Nak on the keyword uses.

This should really just parse the input file whenever TYPE is found
via some fixed format and save the verbose description after that.

Use .rst Field Lists instead, and ideally, keep the list in alphabetic
order or group by similar use.

https://docutils.sourceforge.io/docs/user/rst/quickref.html#field-lists

e.g.:

:LINE_SPACING:
	Vertical space is wasted given the limited number of lines an
	editor window can display when multiple blank lines are used.

:SPACING:
	Whitespace style used in the kernel sources is described in
	ref:`Documentation/process/Coding-Style.rst section 3.1.

:TRAILING_WHITESPACE:
	Trailing whitespace should always be removed.
	Some editors highlight the trailing whitespace and cause visual
	distractions when editing files.

etc...

> @@ -2185,6 +2235,11 @@ sub report {
>  		splice(@lines, 1, 1);
>  		$output = join("\n", @lines);
>  	}
> +
> +	if ($verbose && !$terse &&
> +	    exists $verbose_messages{$type}) {
> +		$output .= $verbose_messages{$type} . "\n\n";
> +	}
>  	$output = (split('\n', $output))[0] . "\n" if ($terse);

Don't use unnecessary multiple tests of the same object, just reorder
the code instead.  And also please use c-style function parentheses
rather than bare tests.

	if ($terse) {
		$output = ...
	} elsif ($verbose && exists($verbose_messages{$type})) {
		$output .= ...
	}



_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH RFC 1/3] checkpatch: add verbose mode

[Dwaipayan Ray]

On Wed, Jan 27, 2021 at 1:41 AM Joe Perches <joe@perches.com> wrote:
>
> On Wed, 2021-01-27 at 00:05 +0530, Dwaipayan Ray wrote:
> > Add a new verbose mode to checkpatch.pl to emit additional verbose
> > test descriptions.
> >
> > The verbose mode is optional and can be enabled by the flag
> > --verbose.
> >
> > The test descriptions are itself loaded from the checkpatch
>
> descriptions are themselves, but themselves is unnecessary.
>
> The verbose descriptions are read from Documentation/dev-tools/checkpatch.rst
>
> > documentation file at Documentation/dev-tools/checkpatch.rst.
> > The descriptions in the documentation are in a specified format
> > enclosed within .. CHECKPATCH_START and .. CHECKPATCH_END labels.
> >
> > This serves a dual purpose as an external documentation to checkpatch
> > as well as enables flawless integration of the verbose mode.
>
> Using 'flawless' when describing code or documentation generally isn't true.
>
> > A subtle example of the format is as follows:
>
> What is subtle about an example?
>
> If there is something subtle about an example, there's also something
> wrong with the example.
>
> > Documentation/dev-tools/checkpatch.rst:
> >
> > .. CHECKPATCH_START
>
> Nak on the keyword uses.
>
> This should really just parse the input file whenever TYPE is found
> via some fixed format and save the verbose description after that.
>
> Use .rst Field Lists instead, and ideally, keep the list in alphabetic
> order or group by similar use.
>
> https://docutils.sourceforge.io/docs/user/rst/quickref.html#field-lists
>
> e.g.:
>
> :LINE_SPACING:
>         Vertical space is wasted given the limited number of lines an
>         editor window can display when multiple blank lines are used.
>
> :SPACING:
>         Whitespace style used in the kernel sources is described in
>         ref:`Documentation/process/Coding-Style.rst section 3.1.
>
> :TRAILING_WHITESPACE:
>         Trailing whitespace should always be removed.
>         Some editors highlight the trailing whitespace and cause visual
>         distractions when editing files.
>
> etc...
>
> > @@ -2185,6 +2235,11 @@ sub report {
> >               splice(@lines, 1, 1);
> >               $output = join("\n", @lines);
> >       }
> > +
> > +     if ($verbose && !$terse &&
> > +         exists $verbose_messages{$type}) {
> > +             $output .= $verbose_messages{$type} . "\n\n";
> > +     }
> >       $output = (split('\n', $output))[0] . "\n" if ($terse);
>
> Don't use unnecessary multiple tests of the same object, just reorder
> the code instead.  And also please use c-style function parentheses
> rather than bare tests.
>
>         if ($terse) {
>                 $output = ...
>         } elsif ($verbose && exists($verbose_messages{$type})) {
>                 $output .= ...
>         }
>

Thanks for the pointers!
Also for the output part can we do something to make the text
look a bit more nice? I think some of the verbose descriptions
can go a bit long.

Also will the verbose descriptions be limited to say single
paragraphs? If there are multiple paragraphs then the output
does appear a bit messy.

Thanks,
Dwaipayan.

Re: [Linux-kernel-mentees] [PATCH RFC 1/3] checkpatch: add verbose mode

[Dwaipayan Ray]

On Wed, Jan 27, 2021 at 1:41 AM Joe Perches <joe@perches.com> wrote:
>
> On Wed, 2021-01-27 at 00:05 +0530, Dwaipayan Ray wrote:
> > Add a new verbose mode to checkpatch.pl to emit additional verbose
> > test descriptions.
> >
> > The verbose mode is optional and can be enabled by the flag
> > --verbose.
> >
> > The test descriptions are itself loaded from the checkpatch
>
> descriptions are themselves, but themselves is unnecessary.
>
> The verbose descriptions are read from Documentation/dev-tools/checkpatch.rst
>
> > documentation file at Documentation/dev-tools/checkpatch.rst.
> > The descriptions in the documentation are in a specified format
> > enclosed within .. CHECKPATCH_START and .. CHECKPATCH_END labels.
> >
> > This serves a dual purpose as an external documentation to checkpatch
> > as well as enables flawless integration of the verbose mode.
>
> Using 'flawless' when describing code or documentation generally isn't true.
>
> > A subtle example of the format is as follows:
>
> What is subtle about an example?
>
> If there is something subtle about an example, there's also something
> wrong with the example.
>
> > Documentation/dev-tools/checkpatch.rst:
> >
> > .. CHECKPATCH_START
>
> Nak on the keyword uses.
>
> This should really just parse the input file whenever TYPE is found
> via some fixed format and save the verbose description after that.
>
> Use .rst Field Lists instead, and ideally, keep the list in alphabetic
> order or group by similar use.
>
> https://docutils.sourceforge.io/docs/user/rst/quickref.html#field-lists
>
> e.g.:
>
> :LINE_SPACING:
>         Vertical space is wasted given the limited number of lines an
>         editor window can display when multiple blank lines are used.
>
> :SPACING:
>         Whitespace style used in the kernel sources is described in
>         ref:`Documentation/process/Coding-Style.rst section 3.1.
>
> :TRAILING_WHITESPACE:
>         Trailing whitespace should always be removed.
>         Some editors highlight the trailing whitespace and cause visual
>         distractions when editing files.
>
> etc...
>
> > @@ -2185,6 +2235,11 @@ sub report {
> >               splice(@lines, 1, 1);
> >               $output = join("\n", @lines);
> >       }
> > +
> > +     if ($verbose && !$terse &&
> > +         exists $verbose_messages{$type}) {
> > +             $output .= $verbose_messages{$type} . "\n\n";
> > +     }
> >       $output = (split('\n', $output))[0] . "\n" if ($terse);
>
> Don't use unnecessary multiple tests of the same object, just reorder
> the code instead.  And also please use c-style function parentheses
> rather than bare tests.
>
>         if ($terse) {
>                 $output = ...
>         } elsif ($verbose && exists($verbose_messages{$type})) {
>                 $output .= ...
>         }
>

Thanks for the pointers!
Also for the output part can we do something to make the text
look a bit more nice? I think some of the verbose descriptions
can go a bit long.

Also will the verbose descriptions be limited to say single
paragraphs? If there are multiple paragraphs then the output
does appear a bit messy.

Thanks,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH RFC 1/3] checkpatch: add verbose mode

[Joe Perches]

On Wed, 2021-01-27 at 01:57 +0530, Dwaipayan Ray wrote:
> On Wed, Jan 27, 2021 at 1:41 AM Joe Perches <joe@perches.com> wrote:
> > On Wed, 2021-01-27 at 00:05 +0530, Dwaipayan Ray wrote:
> > > Add a new verbose mode to checkpatch.pl to emit additional verbose
> > > test descriptions.
> > > 
> > > The verbose mode is optional and can be enabled by the flag
> > > --verbose.
[]
> > > Documentation/dev-tools/checkpatch.rst:
> > > 
> > > .. CHECKPATCH_START
> > 
> > Nak on the keyword uses.
> > 
> > This should really just parse the input file whenever TYPE is found
> > via some fixed format and save the verbose description after that.
> > 
> > Use .rst Field Lists instead, and ideally, keep the list in alphabetic
> > order or group by similar use.
> > 
> > https://docutils.sourceforge.io/docs/user/rst/quickref.html#field-lists
> > 
> > e.g.:
> > 
> > :LINE_SPACING:
> >         Vertical space is wasted given the limited number of lines an
> >         editor window can display when multiple blank lines are used.
> > 
> > :SPACING:
> >         Whitespace style used in the kernel sources is described in
> >         ref:`Documentation/process/Coding-Style.rst section 3.1.
> > 
> > :TRAILING_WHITESPACE:
> >         Trailing whitespace should always be removed.
> >         Some editors highlight the trailing whitespace and cause visual
> >         distractions when editing files.
> > 
> > etc...
[]
> for the output part can we do something to make the text
> look a bit more nice? I think some of the verbose descriptions
> can go a bit long.

Which is why verbose should be optional.

> Also will the verbose descriptions be limited to say single
> paragraphs?

Ideally, no.

> If there are multiple paragraphs then the output
> does appear a bit messy.

I fail to see how that's a problem but play with it and see what
you can do.

Re: [Linux-kernel-mentees] [PATCH RFC 1/3] checkpatch: add verbose mode

[Joe Perches]

On Wed, 2021-01-27 at 01:57 +0530, Dwaipayan Ray wrote:
> On Wed, Jan 27, 2021 at 1:41 AM Joe Perches <joe@perches.com> wrote:
> > On Wed, 2021-01-27 at 00:05 +0530, Dwaipayan Ray wrote:
> > > Add a new verbose mode to checkpatch.pl to emit additional verbose
> > > test descriptions.
> > > 
> > > The verbose mode is optional and can be enabled by the flag
> > > --verbose.
[]
> > > Documentation/dev-tools/checkpatch.rst:
> > > 
> > > .. CHECKPATCH_START
> > 
> > Nak on the keyword uses.
> > 
> > This should really just parse the input file whenever TYPE is found
> > via some fixed format and save the verbose description after that.
> > 
> > Use .rst Field Lists instead, and ideally, keep the list in alphabetic
> > order or group by similar use.
> > 
> > https://docutils.sourceforge.io/docs/user/rst/quickref.html#field-lists
> > 
> > e.g.:
> > 
> > :LINE_SPACING:
> >         Vertical space is wasted given the limited number of lines an
> >         editor window can display when multiple blank lines are used.
> > 
> > :SPACING:
> >         Whitespace style used in the kernel sources is described in
> >         ref:`Documentation/process/Coding-Style.rst section 3.1.
> > 
> > :TRAILING_WHITESPACE:
> >         Trailing whitespace should always be removed.
> >         Some editors highlight the trailing whitespace and cause visual
> >         distractions when editing files.
> > 
> > etc...
[]
> for the output part can we do something to make the text
> look a bit more nice? I think some of the verbose descriptions
> can go a bit long.

Which is why verbose should be optional.

> Also will the verbose descriptions be limited to say single
> paragraphs?

Ideally, no.

> If there are multiple paragraphs then the output
> does appear a bit messy.

I fail to see how that's a problem but play with it and see what
you can do.


_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[PATCH RFC 2/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

Add documentation for kernel script checkpatch.pl.
This documentation is also parsed by checkpatch to
enable a verbose mode.

The test descriptions are potentially incomplete
and will be added over time to document all the
message types in checkpatch.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 Documentation/dev-tools/checkpatch.rst | 283 +++++++++++++++++++++++++
 1 file changed, 283 insertions(+)
 create mode 100644 Documentation/dev-tools/checkpatch.rst

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
new file mode 100644
index 000000000000..b0f1cab108c8
--- /dev/null
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -0,0 +1,283 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+==========
+Checkpatch
+==========
+
+This document describes the kernel script checkpatch.pl.
+
+.. Table of Contents
+
+	=== 1 Introduction
+	=== 2 Options
+	=== 3 Message Levels
+  === 4 Type Descriptions
+
+1 Introduction
+--------------
+
+Checkpatch (scripts/checkpatch.pl) is a perl script which checks for trivial style
+violations in patches and optionally corrects them.  Checkpatch can also be run on
+file contexts and without the kernel tree.
+
+It should be noted that checkpatch may not be always right.  At times the human
+judgement should take preference over what checkpatch has to say.  If your code looks
+better with the violations, then its probably best left alone.
+
+
+2 Options
+---------
+
+This section will describe the options checkpatch can be run with.
+
+Usage::
+
+  ./scripts/checkpatch.pl [OPTION]... [FILE]...
+
+Available options:
+
+ - -q,  --quiet
+
+   Enable quiet mode.  All information messages are disabled.  Only the
+   messages and a summary report is output.
+
+ - --no-tree
+
+   Run checkpatch without the kernel tree.
+
+ - --no-signoff
+
+   Disable the 'Signed-off-by' line check.  The sign-off is a simple line at
+   the end of the explanation for the patch, which certifies that you wrote it
+   or otherwise have the right to pass it on as an open-source patch.
+
+   Example::
+
+	 Signed-off-by: Random J Developer <random@developer.example.org>
+
+   Setting this flag effectively stops a message for a missing signed-off-by line
+   in a patch context.
+
+ - --patch
+
+   Treat FILE as a patch.  This is the default option and need not be
+   explicitly specified.
+
+ - --emacs
+
+   Set output to emacs compile window format.  This allows emacs users to jump
+   from the error in the compile window directly to the offending line in the patch.
+
+ - --terse
+
+   Output only one line per report.
+
+ - --showfile
+
+   Show the diffed file position instead of the input file position.
+
+ - -g,  --git
+
+   Treat FILE as a single commit or a git revision range.
+
+   Single commit with:
+
+   - <rev>
+   - <rev>^
+   - <rev>~n
+
+   Multiple commits with:
+
+   - <rev1>..<rev2>
+   - <rev1>...<rev2>
+   - <rev>-<count>
+
+ - -f,  --file
+
+   Treat FILE as a regular source file.  This option must be used when running
+   checkpatch on source files in the kernel.
+
+ - --subjective,  --strict
+
+   Enable stricter tests in checkpatch.  By default the tests emitted as CHECK
+   do not activate by default.  Use this flag to activate the CHECK tests.
+
+ - --list-types
+
+   Every message emitted by checkpatch has an associated TYPE.  Add this flag to
+   display all the types in checkpatch.
+
+   Note that when this flag is active, checkpatch does not read the input FILE, and
+   no message is emitted.  Only a list of types in checkpatch is output.
+
+ - --types TYPE(,TYPE2...)
+
+   Only display messages with the given types.
+
+   Example::
+
+     ./scripts/checkpatch.pl mypatch.patch --types EMAIL_SUBJECT,NO_AUTHOR_SIGN_OFF
+
+ - --ignore TYPE(,TYPE2...)
+
+   Strip off messages with the given types.
+
+   Example::
+
+     ./scripts/checkpatch.pl mypatch.patch --ignore EMAIL_SUBJECT,NO_AUTHOR_SIGN_OFF
+
+ - --show-types
+
+   By default checkpatch doesn't display the type associated with the messages.
+   Set this flag to show the message type in the output.
+
+ - --max-line-length=n
+
+   Set the max line length (default 100).  On exceeding the given length, a message
+   is emitted.
+
+   The message level is different for patch and file contexts.  For patches, a WARNING is
+   emitted.  While a milder CHECK is emitted for files.  So for file contexts, the --strict
+   flag must also be enabled.
+
+ - --min-conf-desc-length=n
+
+   Set the min description length, if shorter, warn.
+
+ - --tab-size=n
+
+   Set the number of spaces for tab (default 8).
+
+ - --root=PATH
+
+   PATH to the kernel tree root.
+
+   This option must be specified when invoking checkpatch from outside
+   the kernel root.
+
+ - --no-summary
+
+   Suppress the per file summary.
+
+ - --mailback
+
+   Only produce a report in case of Warnings or Errors.  Milder Checks are
+   excluded from this.
+
+ - --summary-file
+
+   Include the filename in summary.
+
+ - --debug KEY=[0|1]
+
+   Turn on/off debugging of KEY, where KEY is one of 'values', 'possible',
+   'type', and 'attr' (default is all off).
+
+ - --fix
+
+   This is an EXPERIMENTAL feature.  If correctable errors exists, a file
+   <inputfile>.EXPERIMENTAL-checkpatch-fixes is created which has the
+   automatically fixable errors corrected.
+
+ - --fix-inplace
+
+   EXPERIMENTAL - Similar to --fix but the input file is overwritten with fixes.
+
+   DO NOT USE this flag unless you are absolutely sure and you have a backup in place.
+
+ - --ignore-perl-version
+
+   Override checking of perl version.  Runtime errors maybe encountered after
+   enabling this flag if the perl version does not meet the minimum specified.
+
+ - --codespell
+
+   Use the codespell dictionary for checking spelling errors.
+
+ - --codespellfile
+
+   Use the specified codespell file.  Default is '/usr/share/codespell/dictionary.txt'.
+
+ - --typedefsfile
+
+   Read additional types from this file.
+
+ - --color[=WHEN]
+
+   Use colors 'always', 'never', or only when output is a terminal ('auto').
+   Default is 'auto'.
+
+ - --kconfig-prefix=WORD
+
+   Use WORD as a prefix for Kconfig symbols (default is `CONFIG_`).
+
+ - -h, --help, --version
+
+   Display the help text.
+
+3 Message Levels
+----------------
+
+Messages in checkpatch are divided into three levels. The levels of messages in
+checkpatch denote the severity of the error. They are:
+
+ - ERROR
+
+   This is the most strict level.  Messages of type ERROR must be taken
+   seriously as they denote things that are very likely to be wrong.
+
+ - WARNING
+
+   This is the next stricter level.  Messages of type WARNING requires a
+   more careful review.  But it is milder than an ERROR.
+
+ - CHECK
+
+   This is the mildest level.  These are things which may require some thought.
+
+4 Type Descriptions
+-------------------
+
+This section contains a description of all the message types in checkpatch.
+
+.. Types in this section are also parsed by checkpatch.
+.. Please keep the types sorted alphabetically.
+.. CHECKPATCH_START
+
+EMAIL_SUBJECT
+
+  The subject line should describe the change not the tool used to find
+  the change.  Consider a patch entitled:
+
+  'Fix warning detected by tool <some tool>'.
+
+  This is potentially a bad practice and the actual changes should be
+  summarised in the subject line.
+
+MISSING_SIGN_OFF
+
+  The patch is missing a Signed-off-by line.  This error must be taken
+  care of and a Signed-off-by line should be added according to
+  Developer's certificate of Origin.
+
+NO_AUTHOR_SIGN_OFF
+
+  The author of the patch has not signed off the patch.  It is required
+  that a simple sign off line should be present at the end of explanation
+  of the patch to denote that the author has written it or otherwise has
+  the rights to pass it on as an open source patch.
+
+  Sometimes this particular warning can also occur when both email address
+  and name of the author do not match the sign off line because checkpatch
+  has no mechanism to say if it is the same person.
+
+  Consider::
+
+    From: John Doe <john.doe@example.com>
+    ...
+    Signed-off-by: J. Doe <john@example2.com>
+
+  While these may point that both the persons are same, checkpatch cannot
+  understand that and in such cases this warning can be ignored.
+
+.. CHECKPATCH_END
-- 
2.30.0

[Linux-kernel-mentees] [PATCH RFC 2/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

Add documentation for kernel script checkpatch.pl.
This documentation is also parsed by checkpatch to
enable a verbose mode.

The test descriptions are potentially incomplete
and will be added over time to document all the
message types in checkpatch.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 Documentation/dev-tools/checkpatch.rst | 283 +++++++++++++++++++++++++
 1 file changed, 283 insertions(+)
 create mode 100644 Documentation/dev-tools/checkpatch.rst

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
new file mode 100644
index 000000000000..b0f1cab108c8
--- /dev/null
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -0,0 +1,283 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+==========
+Checkpatch
+==========
+
+This document describes the kernel script checkpatch.pl.
+
+.. Table of Contents
+
+	=== 1 Introduction
+	=== 2 Options
+	=== 3 Message Levels
+  === 4 Type Descriptions
+
+1 Introduction
+--------------
+
+Checkpatch (scripts/checkpatch.pl) is a perl script which checks for trivial style
+violations in patches and optionally corrects them.  Checkpatch can also be run on
+file contexts and without the kernel tree.
+
+It should be noted that checkpatch may not be always right.  At times the human
+judgement should take preference over what checkpatch has to say.  If your code looks
+better with the violations, then its probably best left alone.
+
+
+2 Options
+---------
+
+This section will describe the options checkpatch can be run with.
+
+Usage::
+
+  ./scripts/checkpatch.pl [OPTION]... [FILE]...
+
+Available options:
+
+ - -q,  --quiet
+
+   Enable quiet mode.  All information messages are disabled.  Only the
+   messages and a summary report is output.
+
+ - --no-tree
+
+   Run checkpatch without the kernel tree.
+
+ - --no-signoff
+
+   Disable the 'Signed-off-by' line check.  The sign-off is a simple line at
+   the end of the explanation for the patch, which certifies that you wrote it
+   or otherwise have the right to pass it on as an open-source patch.
+
+   Example::
+
+	 Signed-off-by: Random J Developer <random@developer.example.org>
+
+   Setting this flag effectively stops a message for a missing signed-off-by line
+   in a patch context.
+
+ - --patch
+
+   Treat FILE as a patch.  This is the default option and need not be
+   explicitly specified.
+
+ - --emacs
+
+   Set output to emacs compile window format.  This allows emacs users to jump
+   from the error in the compile window directly to the offending line in the patch.
+
+ - --terse
+
+   Output only one line per report.
+
+ - --showfile
+
+   Show the diffed file position instead of the input file position.
+
+ - -g,  --git
+
+   Treat FILE as a single commit or a git revision range.
+
+   Single commit with:
+
+   - <rev>
+   - <rev>^
+   - <rev>~n
+
+   Multiple commits with:
+
+   - <rev1>..<rev2>
+   - <rev1>...<rev2>
+   - <rev>-<count>
+
+ - -f,  --file
+
+   Treat FILE as a regular source file.  This option must be used when running
+   checkpatch on source files in the kernel.
+
+ - --subjective,  --strict
+
+   Enable stricter tests in checkpatch.  By default the tests emitted as CHECK
+   do not activate by default.  Use this flag to activate the CHECK tests.
+
+ - --list-types
+
+   Every message emitted by checkpatch has an associated TYPE.  Add this flag to
+   display all the types in checkpatch.
+
+   Note that when this flag is active, checkpatch does not read the input FILE, and
+   no message is emitted.  Only a list of types in checkpatch is output.
+
+ - --types TYPE(,TYPE2...)
+
+   Only display messages with the given types.
+
+   Example::
+
+     ./scripts/checkpatch.pl mypatch.patch --types EMAIL_SUBJECT,NO_AUTHOR_SIGN_OFF
+
+ - --ignore TYPE(,TYPE2...)
+
+   Strip off messages with the given types.
+
+   Example::
+
+     ./scripts/checkpatch.pl mypatch.patch --ignore EMAIL_SUBJECT,NO_AUTHOR_SIGN_OFF
+
+ - --show-types
+
+   By default checkpatch doesn't display the type associated with the messages.
+   Set this flag to show the message type in the output.
+
+ - --max-line-length=n
+
+   Set the max line length (default 100).  On exceeding the given length, a message
+   is emitted.
+
+   The message level is different for patch and file contexts.  For patches, a WARNING is
+   emitted.  While a milder CHECK is emitted for files.  So for file contexts, the --strict
+   flag must also be enabled.
+
+ - --min-conf-desc-length=n
+
+   Set the min description length, if shorter, warn.
+
+ - --tab-size=n
+
+   Set the number of spaces for tab (default 8).
+
+ - --root=PATH
+
+   PATH to the kernel tree root.
+
+   This option must be specified when invoking checkpatch from outside
+   the kernel root.
+
+ - --no-summary
+
+   Suppress the per file summary.
+
+ - --mailback
+
+   Only produce a report in case of Warnings or Errors.  Milder Checks are
+   excluded from this.
+
+ - --summary-file
+
+   Include the filename in summary.
+
+ - --debug KEY=[0|1]
+
+   Turn on/off debugging of KEY, where KEY is one of 'values', 'possible',
+   'type', and 'attr' (default is all off).
+
+ - --fix
+
+   This is an EXPERIMENTAL feature.  If correctable errors exists, a file
+   <inputfile>.EXPERIMENTAL-checkpatch-fixes is created which has the
+   automatically fixable errors corrected.
+
+ - --fix-inplace
+
+   EXPERIMENTAL - Similar to --fix but the input file is overwritten with fixes.
+
+   DO NOT USE this flag unless you are absolutely sure and you have a backup in place.
+
+ - --ignore-perl-version
+
+   Override checking of perl version.  Runtime errors maybe encountered after
+   enabling this flag if the perl version does not meet the minimum specified.
+
+ - --codespell
+
+   Use the codespell dictionary for checking spelling errors.
+
+ - --codespellfile
+
+   Use the specified codespell file.  Default is '/usr/share/codespell/dictionary.txt'.
+
+ - --typedefsfile
+
+   Read additional types from this file.
+
+ - --color[=WHEN]
+
+   Use colors 'always', 'never', or only when output is a terminal ('auto').
+   Default is 'auto'.
+
+ - --kconfig-prefix=WORD
+
+   Use WORD as a prefix for Kconfig symbols (default is `CONFIG_`).
+
+ - -h, --help, --version
+
+   Display the help text.
+
+3 Message Levels
+----------------
+
+Messages in checkpatch are divided into three levels. The levels of messages in
+checkpatch denote the severity of the error. They are:
+
+ - ERROR
+
+   This is the most strict level.  Messages of type ERROR must be taken
+   seriously as they denote things that are very likely to be wrong.
+
+ - WARNING
+
+   This is the next stricter level.  Messages of type WARNING requires a
+   more careful review.  But it is milder than an ERROR.
+
+ - CHECK
+
+   This is the mildest level.  These are things which may require some thought.
+
+4 Type Descriptions
+-------------------
+
+This section contains a description of all the message types in checkpatch.
+
+.. Types in this section are also parsed by checkpatch.
+.. Please keep the types sorted alphabetically.
+.. CHECKPATCH_START
+
+EMAIL_SUBJECT
+
+  The subject line should describe the change not the tool used to find
+  the change.  Consider a patch entitled:
+
+  'Fix warning detected by tool <some tool>'.
+
+  This is potentially a bad practice and the actual changes should be
+  summarised in the subject line.
+
+MISSING_SIGN_OFF
+
+  The patch is missing a Signed-off-by line.  This error must be taken
+  care of and a Signed-off-by line should be added according to
+  Developer's certificate of Origin.
+
+NO_AUTHOR_SIGN_OFF
+
+  The author of the patch has not signed off the patch.  It is required
+  that a simple sign off line should be present at the end of explanation
+  of the patch to denote that the author has written it or otherwise has
+  the rights to pass it on as an open source patch.
+
+  Sometimes this particular warning can also occur when both email address
+  and name of the author do not match the sign off line because checkpatch
+  has no mechanism to say if it is the same person.
+
+  Consider::
+
+    From: John Doe <john.doe@example.com>
+    ...
+    Signed-off-by: J. Doe <john@example2.com>
+
+  While these may point that both the persons are same, checkpatch cannot
+  understand that and in such cases this warning can be ignored.
+
+.. CHECKPATCH_END
-- 
2.30.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[PATCH RFC 3/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

Link checkpatch.rst to the dev-tools index for sphinx.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 Documentation/dev-tools/index.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst
index 1b1cf4f5c9d9..43d28998118b 100644
--- a/Documentation/dev-tools/index.rst
+++ b/Documentation/dev-tools/index.rst
@@ -14,6 +14,7 @@ whole; patches welcome!
 .. toctree::
    :maxdepth: 2
 
+   checkpatch
    coccinelle
    sparse
    kcov
-- 
2.30.0

[Linux-kernel-mentees] [PATCH RFC 3/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

Link checkpatch.rst to the dev-tools index for sphinx.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 Documentation/dev-tools/index.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst
index 1b1cf4f5c9d9..43d28998118b 100644
--- a/Documentation/dev-tools/index.rst
+++ b/Documentation/dev-tools/index.rst
@@ -14,6 +14,7 @@ whole; patches welcome!
 .. toctree::
    :maxdepth: 2
 
+   checkpatch
    coccinelle
    sparse
    kcov
-- 
2.30.0

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees