Re: [PATCH] scripts: kernel-doc: allow passing desired Sphinx C domain dialect

[Mauro Carvalho Chehab <mchehab+huawei@kernel.org>]

Em Tue, 6 Oct 2020 08:01:34 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:

> On Tue, 6 Oct 2020 08:42:07 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> 
> > 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.  
> 
> Maybe...someday we may need it, knowing how the Sphinx folks approach
> compatibility, but I guess we can always add it then if so.
> 
> > The change would be trivial, although the regex will become even more
> > harder to read ;-)  
> 
> 	^(\d+)(\.(\d+)){,2}
> 
> ?  (untested, of course)

Didn't work (perl complains about its syntax), and neither:

	^(\d+)(\.(\d+)){0,2}

I also tried this:

	^(\d+)(?:\.(\d+)){0,2}

But both ends misplacing the second group information when
x.y.z is used.

On the tests I did, this is the one that worked fine:

	if ($ver_string =~ m/^(\d+)(?:\.(\d+))?(?:\.(\d+))?/) {
	    $sphinx_major = $1;
	    if (defined($2)) {
		$sphinx_minor = $2;
	    } else {
		$sphinx_minor = 0;
	    }
	    if (defined($3)) {
		$sphinx_patch = $3
	    } else {
		$sphinx_patch = 0;
	    }

Or the alternative one, using substrings:

	if ($ver_string =~ m/^(\d+)(\.\d+)?(\.\d+)?/) {
	    $sphinx_major = $1;
	    if (defined($2)) {
		$sphinx_minor = substr($2,1);
	    } else {
		$sphinx_minor = 0;
	    }
	    if (defined($3)) {
		$sphinx_patch = substr($3,1)
	    } else {
		$sphinx_patch = 0;
	    }
	}

I'll keep the last one, as it is likely simpler to understand.

> 
> > [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.   
> 
> 3.0 might just be skippable at this point, methinks.  

Yeah, agreed. 

I'll fold the latest version of this patch with the following diff.

Thanks,
Mauro

index 297312824d26..c8f6b11d5da1 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -466,11 +466,15 @@ 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;
+	    if (defined($2)) {
+		$sphinx_minor = substr($2,1);
+	    } else {
+		$sphinx_minor = 0;
+	    }
 	    if (defined($3)) {
-		$sphinx_patch = substr($3,1);
+		$sphinx_patch = substr($3,1)
 	    } else {
 		$sphinx_patch = 0;
 	    }
@@ -2368,7 +2372,10 @@ sub process_file($) {
 }
 
 
-get_sphinx_version() if (!$sphinx_major);
+if ($output_mode eq "rst") {
+	get_sphinx_version() if (!$sphinx_major);
+}
+
 $kernelversion = get_kernel_version();
 
 # generate a sequence of code that will splice in highlighting information