lkml.org 
[lkml]   [2020]   [Oct]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] scripts: kernel-doc: allow passing desired Sphinx C domain dialect
Em Mon, 5 Oct 2020 10:17:36 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:

> On Sun, 4 Oct 2020 10:02:03 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
>
> > When kernel-doc is called via kerneldoc.py, there's no need to
> > auto-detect the Sphinx version, as the Sphinx module already
> > knows it. So, add an optional parameter to allow changing the
> > Sphinx dialect.
> >
> > As kernel-doc can also be manually called, keep the auto-detection
> > logic if the parameter was not specified. On such case, emit
> > a warning if sphinx-build can't be found at PATH.
> >
> > Suggested-by: Jonathan Corbet <corbet@lwn.net>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > ---
> > Documentation/sphinx/kerneldoc.py | 5 ++++
> > scripts/kernel-doc | 40 ++++++++++++++++++++++++-------
> > 2 files changed, 37 insertions(+), 8 deletions(-)
>
> So I'm glad to see this. Still not fully sold on the autodetection, but if
> we don't actually use it, maybe I can live with it :)
>
> One little nit:
>
> > diff --git a/Documentation/sphinx/kerneldoc.py b/Documentation/sphinx/kerneldoc.py
> > index 233f610539f0..e9857ab904f1 100644
> > --- a/Documentation/sphinx/kerneldoc.py
> > +++ b/Documentation/sphinx/kerneldoc.py
> > + } elsif ($cmd eq "sphinx-version") {
> > + my $ver_string = shift @ARGV;
> > + if ($ver_string =~ m/^(\d+)\.(\d+)\.(\d+)/) {
> > + $sphinx_major = $1;
> > + $sphinx_minor = $2;
> > + $sphinx_patch = $3;
> > + } else {
> > + die "Sphinx version should be at major.minor.patch format\n";
> > + }
>
> Can we allow just major.minor, with patch defaulting to zero? People
> passing this by hand may not want to look up their patch version every
> time, and I doubt it will ever matter...

Sure. It should be easy to make the third argument optional, although
the regex will be a little more harder to understand.

Something like this should do the trick:

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 01efb0afb8c2..104d79949a8a 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -466,12 +466,16 @@ while ($ARGV[0] =~ m/^--?(.*)/) {
$show_not_found = 1; # A no-op but don't fail
} elsif ($cmd eq "sphinx-version") {
my $ver_string = shift @ARGV;
- if ($ver_string =~ m/^(\d+)\.(\d+)\.(\d+)/) {
+ if ($ver_string =~ m/^(\d+)\.(\d+)(?:\.?(\d+)?)/) {
$sphinx_major = $1;
$sphinx_minor = $2;
- $sphinx_patch = $3;
+ if ($3) {
+ $sphinx_patch = $3;
+ } else {
+ $sphinx_patch = 0;
+ }
} else {
- die "Sphinx version should be at major.minor.patch format\n";
+ die "Sphinx version should either major.minor or major.minor.patch format\n";
}
} else {
# Unknown argument
As right now we don't support Sphinx version 3.0[1], we're actually using just
$sphinx_major. So, I'm wonder if it would make sense to also make <minor>
optional.

The change would be trivial, although the regex will become even more
harder to read ;-)

[1] not sure how valuable would be adding support for Sphinx 3.0. While
I didn't make any tests, I'm strongly suspecting that, with the approach
we took for backward/forward compatibility, adding support for it
would mean to just do a trivial change at cdomain.py by applying a
patch that Markus did replacing a regex function that doesn't exist
anymore at Sphinx API and emulating C namespace with the logic I
already implemented.

I guess I'll give it a try anyway, as it seems weird to have a gap
in the middle of the supported versions.


Thanks,
Mauro

\
 
 \ /
  Last update: 2020-10-06 08:42    [W:0.308 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site