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

[Dwaipayan Ray]

On Wed, Jan 27, 2021 at 1:41 AM Joe Perches <joe@xxxxxxxxxxx> 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.