Date:   Mon, 3 Jan 2022 21:40:35 +0100
From:   Mauro Carvalho Chehab <mchehab@kernel.org>
To:     Jonathan Corbet <corbet@lwn.net>
Cc:     Tomasz =?UTF-8?B?V2FybmllxYLFgm8=?= <tomasz.warniello@gmail.com>,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] scripts: kernel-doc: transform documentation into POD
Message-ID: <20220103214023.15cd2570@coco.lan>
In-Reply-To: <87h7b8cfg0.fsf@meer.lwn.net>
References: <20211209225549.69010-1-tomasz.warniello@gmail.com>
        <87h7b8cfg0.fsf@meer.lwn.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Em Thu, 16 Dec 2021 16:12:15 -0700
Jonathan Corbet <corbet@lwn.net> escreveu:

> Tomasz Warnie=C5=82=C5=82o <tomasz.warniello@gmail.com> writes:
>=20
> > The only change in the script execution flow is the replacement
> > of the 'usage' function with the native core Perl 'pod2usage'.
> >
> > This entails:
> > - an overall documentation restructuring
> > - addition of a synopsis
> >
> > Otherwise my intervention is minimal:
> > - a few tiny language, formatting and spacing corrections
> > - a few missing bits added in the command syntax description
> > - adding subsections in the "FORMAT OF COMMENTS" section
> > - alphabetical sorting within OPTIONS subections =20
>=20
> So I think that this is generally a good thing, but I do have some
> quibbles.  Starting with the above, which is a pretty clear violation of
> the "each patch does one thing" rule.  Please separate out your changes
> into separate patches so that they are more easily reviewed.

I almost did that in the past, but due to a different rationale ;-)

Besides the points that Jonathan already mentioned, I'd like to add
one additional request...

Pod is very useful when associated with Getopt, e. g.:

	use Getopt::Long;
	use Pod::Usage;

In a similar way to scripts/get_abi.pl (and scripts/get_feat.pl).

This simplifies a lot the parameter handling, as it would
be like:

	GetOptions(
		"debug=3Di" =3D> \$debug,
		"enable-lineno" =3D> \$enable_lineno,
		"rst-source!" =3D> \$description_is_rst,
		"dir=3Ds" =3D> \$prefix,
		'help|?' =3D> \$help,
		"show-hints" =3D> \$hint,
		"search-string=3Ds" =3D> \$search_string,
		man =3D> \$man
	) or pod2usage(2);

This would make easier to maintain the parameters and the associated
summary help and man-like help. An additional advantage of using Getopt
is that the parameters can be added before or after the file name.

IMO, the best would be to add a patch at the version 2 of this series
in order to use GetOpt too.

> A few other things below...
>=20
> > Finally, the TODO stub evolves into a section:
> > - perldoc request removed
> > - undocumented options added
> >
> > Run `kernel-doc -h` to see the full doc.
> >
> > The TODO suggestion is ancient, thus I can't address its author with
> > a "Suggested-by" tag.
> >
> > Signed-off-by: Tomasz Warnie=C5=82=C5=82o <tomasz.warniello@gmail.com>
> > ---
> >  scripts/kernel-doc | 613 ++++++++++++++++++++++++++++++---------------
> >  1 file changed, 413 insertions(+), 200 deletions(-)
> >
> > diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> > index 3106b7536b89..00c0c7f5ff58 100755
> > --- a/scripts/kernel-doc
> > +++ b/scripts/kernel-doc
> > @@ -4,46 +4,33 @@
> >  use warnings;
> >  use strict;

I would also add:

	BEGIN { $Pod::Usage::Formatter =3D 'Pod::Text::Termcap'; }

at the final version, in order to produce a better output.

Regards,
Mauro


Thanks,
Mauro