2020-12-26 14:09:05

by Nicolai Fischer

[permalink] [raw]
Subject: [PATCH v2 3/4] checkpatch: kconfig: enforce help text indentation

Adds a new warning in case the indentation level of the
first line of a Kconfig help message is not two spaces
higher than the keyword itself.
Blank lines between the message and the help keyword
are ignored.

Co-developed-by: Johannes Czekay <[email protected]>
Signed-off-by: Johannes Czekay <[email protected]>
Signed-off-by: Nicolai Fischer <[email protected]>
---
scripts/checkpatch.pl | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c86a971a3205..aa2205ee9ab8 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3313,6 +3313,8 @@ sub process {
my $f;
my $is_start = 0;
my $is_end = 0;
+ my $help_indent;
+ my $help_stat_real;
for (; $cnt > 0 && defined $lines[$ln - 1]; $ln++) {
$f = $lines[$ln - 1];
$cnt-- if ($lines[$ln - 1] !~ /^-/);
@@ -3323,8 +3325,10 @@ sub process {

if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate|int|hex|string|prompt)\s*(?:["'].*)?$/) {
$is_start = 1;
- } elsif ($lines[$ln - 1] =~ /^\+\s*help$/) {
- $length = -1;
+ } elsif ($lines[$ln - 1] =~ /^\+(\s*)help$/) {
+ $help_indent = $1;
+ $length = 0;
+ next;
}

$f =~ s/^.//;
@@ -3332,6 +3336,13 @@ sub process {
$f =~ s/^\s+//;
next if ($f =~ /^$/);

+ if (defined $help_indent) {
+ if ($lines[$ln - 1] !~ /^\+$help_indent\ {2}\S*/) {
+ $help_stat_real = get_stat_real($ln - 1, $ln);
+ }
+ undef $help_indent;
+ }
+
# This only checks context lines in the patch
# and so hopefully shouldn't trigger false
# positives, even though some of these are
@@ -3347,6 +3358,10 @@ sub process {
WARN("CONFIG_DESCRIPTION",
"please write a paragraph that describes the config symbol fully\n" . $herecurr);
}
+ if ($is_start && $is_end && defined $help_stat_real) {
+ WARN("CONFIG_DESCRIPTION",
+ "please indent the help text two spaces more than the keyword\n" . "$here\n$help_stat_real\n");
+ }
#print "is_start<$is_start> is_end<$is_end> length<$length>\n";
}

--
2.29.2


2020-12-26 15:52:54

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] checkpatch: kconfig: enforce help text indentation

On Sat, 2020-12-26 at 15:05 +0100, Nicolai Fischer wrote:
> Adds a new warning in case the indentation level of the
> first line of a Kconfig help message is not two spaces
> higher than the keyword itself.
> Blank lines between the message and the help keyword
> are ignored.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -3332,6 +3336,13 @@ sub process {
> ? $f =~ s/^\s+//;
> ? next if ($f =~ /^$/);
> ?
>
> + if (defined $help_indent) {
> + if ($lines[$ln - 1] !~ /^\+$help_indent\ {2}\S*/) {
> + $help_stat_real = get_stat_real($ln - 1, $ln);
> + }
> + undef $help_indent;
> + }

This doesn't work if the indent is more than 2 spaces.

$ cat Kconfigtest
menuconfig FOO
bool "Enable foo" if EXPERT
default y
help
Line 1.
Line 2.
Line 3.
Line 4.

$ ./scripts/checkpatch.pl -f Kconfigtest
total: 0 errors, 0 warnings, 10 lines checked

Kconfigtest has no obvious style problems and is ready for submission.

Also, it may be useful to test that the indent after a block
uses a single tab more than the block start.

Look at the first block of block/Kconfig:

The indentation of bool and help uses 7 spaces but the indentation
of the help text uses a tab then 1 space.

It'd be useful to emit a warning for that.

menuconfig BLOCK
bool "Enable the block layer" if EXPERT
default y
select SBITMAP
select SRCU
help
Provide block layer support for the kernel.

Disable this option to remove the block layer support from the
kernel. This may be useful for embedded devices.

If this option is disabled:

- block device files will become unusable
- some filesystems (such as ext3) will become unavailable.

Also, SCSI character devices and USB storage will be disabled since
they make use of various block layer definitions and facilities.

Say Y here unless you know you really don't want to mount disks and
suchlike.