Message-ID: <87r0zmqkao.fsf@intel.com>
Date:   Wed, 05 Oct 2022 19:58:39 +0300
From:   Jani Nikula <jani.nikula@...ux.intel.com>
To:     Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH 5/5] docs: improve the HTML formatting of kerneldoc
 comments

On Tue, 04 Oct 2022, Jonathan Corbet <corbet@....net> wrote:
> Make a few changes to cause functions documented by kerneldoc to stand out
> better in the rendered documentation.  Specifically, change kernel-doc to
> put the description section into a ".. container::" section, then add a bit
> of CSS to indent that section relative to the function prototype (or struct
> or enum definition).  Tweak a few other CSS parameters while in the
> neighborhood to improve the formatting.

Way back I tried to keep the formatting changes minimal to avoid opening
that particular can of worms along with the rest of the Sphinx
transition.

But I do wonder if people find value in repeating e.g. the struct
definitions in the documentation. I'd argue the rendered documentation
is more for an overview, and if you need to know the exact details,
you'll be in the editor typing code and you can look up the actual
definition in source. Having the definition feels maybe a bit excessive.

We also don't use Sphinx C Domain's ".. c:member::" for struct/union
members, or ".. c:enumerator::" for enumeration contants. They provide
arguably nicer rendering out of the box than our stuff.

The Sphinx way to do parameter lists would be field lists i.e. ":param
foo: description". Ditto for return values ":return: description". (Not
saying we should convert the comments, but kernel-doc the script could
emit those.)

Perhaps we'd be better off going towards Sphinx standard usage than
tweaking our own thing?

I'm afraid I don't have the time to work on this. Talk is cheap and all
that. My two cents.

Anyway, here are some examples how this might look like: [1].


BR,
Jani.



[1] https://hawkmoth.readthedocs.io/en/latest/examples.html


>
> Signed-off-by: Jonathan Corbet <corbet@....net>
> ---
>  Documentation/sphinx-static/custom.css | 13 +++++++
>  scripts/kernel-doc                     | 52 ++++++++++++++++----------
>  2 files changed, 45 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/sphinx-static/custom.css b/Documentation/sphinx-static/custom.css
> index c465251e840a..d8823fbbd27b 100644
> --- a/Documentation/sphinx-static/custom.css
> +++ b/Documentation/sphinx-static/custom.css
> @@ -10,3 +10,16 @@ div.body h3 { font-size: 130%; }
>  
>  /* Tighten up the layout slightly */
>  div.body { padding: 0 15px 0 10px; }
> +
> +/* Don't force the document width */
> +div.document { width: auto; max-width: 60em; }
> +
> +/*
> + * Parameters for the display of function prototypes and such included
> + * from C source files.
> + */
> +dl.function, dl.struct, dl.enum { margin-top: 2em; background-color: #ecf0f3; }
> +/* indent lines 2+ of multi-line function prototypes */
> +dl.function dt { margin-left: 10em; text-indent: -10em; }
> +dt.sig-object { font-size: larger; }
> +div.kernelindent { margin-left: 2em; margin-right: 4em; }
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index aea04365bc69..13d42857bce2 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -866,48 +866,53 @@ sub output_function_rst(%) {
>  	print "\n";
>      }
>  
> -    print "**Parameters**\n\n";
> +    #
> +    # Put our descriptive text into a container (thus an HTML <div> to help
> +    # set the function prototypes apart.
> +    #
> +    print ".. container:: kernelindent\n\n";
>      $lineprefix = "  ";
> +    print $lineprefix . "**Parameters**\n\n";
>      foreach $parameter (@{$args{'parameterlist'}}) {
>  	my $parameter_name = $parameter;
>  	$parameter_name =~ s/\[.*//;
>  	$type = $args{'parametertypes'}{$parameter};
>  
>  	if ($type ne "") {
> -	    print "``$type``\n";
> +	    print $lineprefix . "``$type``\n";
>  	} else {
> -	    print "``$parameter``\n";
> +	    print $lineprefix . "``$parameter``\n";
>  	}
>  
>          print_lineno($parameterdesc_start_lines{$parameter_name});
>  
> +	$lineprefix = "    ";
>  	if (defined($args{'parameterdescs'}{$parameter_name}) &&
>  	    $args{'parameterdescs'}{$parameter_name} ne $undescribed) {
>  	    output_highlight_rst($args{'parameterdescs'}{$parameter_name});
>  	} else {
> -	    print "  *undescribed*\n";
> +	    print $lineprefix . "*undescribed*\n";
>  	}
> +	$lineprefix = "  ";
>  	print "\n";
>      }
>  
> -    $lineprefix = $oldprefix;
>      output_section_rst(@_);
> +    $lineprefix = $oldprefix;
>  }
>  
>  sub output_section_rst(%) {
>      my %args = %{$_[0]};
>      my $section;
>      my $oldprefix = $lineprefix;
> -    $lineprefix = "";
>  
>      foreach $section (@{$args{'sectionlist'}}) {
> -	print "**$section**\n\n";
> +	print $lineprefix . "**$section**\n\n";
>          print_lineno($section_start_lines{$section});
>  	output_highlight_rst($args{'sections'}{$section});
>  	print "\n";
>      }
>      print "\n";
> -    $lineprefix = $oldprefix;
>  }
>  
>  sub output_enum_rst(%) {
> @@ -915,6 +920,7 @@ sub output_enum_rst(%) {
>      my ($parameter);
>      my $oldprefix = $lineprefix;
>      my $count;
> +    my $outer;
>  
>      if ($sphinx_major < 3) {
>  	my $name = "enum " . $args{'enum'};
> @@ -924,14 +930,17 @@ sub output_enum_rst(%) {
>  	print "\n\n.. c:enum:: " . $name . "\n\n";
>      }
>      print_lineno($declaration_start_line);
> -    $lineprefix = "   ";
> +    $lineprefix = "  ";
>      output_highlight_rst($args{'purpose'});
>      print "\n";
>  
> -    print "**Constants**\n\n";
> -    $lineprefix = "  ";
> +    print ".. container:: kernelindent\n\n";
> +    $outer = $lineprefix . "  ";
> +    $lineprefix = $outer . "  ";
> +    print $outer . "**Constants**\n\n";
>      foreach $parameter (@{$args{'parameterlist'}}) {
> -	print "``$parameter``\n";
> +	print $outer . "``$parameter``\n";
> +
>  	if ($args{'parameterdescs'}{$parameter} ne $undescribed) {
>  	    output_highlight_rst($args{'parameterdescs'}{$parameter});
>  	} else {
> @@ -939,7 +948,7 @@ sub output_enum_rst(%) {
>  	}
>  	print "\n";
>      }
> -
> +    print "\n";
>      $lineprefix = $oldprefix;
>      output_section_rst(@_);
>  }
> @@ -982,18 +991,19 @@ sub output_struct_rst(%) {
>  	}
>      }
>      print_lineno($declaration_start_line);
> -    $lineprefix = "   ";
> +    $lineprefix = "  ";
>      output_highlight_rst($args{'purpose'});
>      print "\n";
>  
> -    print "**Definition**\n\n";
> -    print "::\n\n";
> +    print ".. container:: kernelindent\n\n";
> +    print $lineprefix . "**Definition**::\n\n";
>      my $declaration = $args{'definition'};
> -    $declaration =~ s/\t/  /g;
> -    print "  " . $args{'type'} . " " . $args{'struct'} . " {\n$declaration  };\n\n";
> +    $lineprefix = $lineprefix . "  ";
> +    $declaration =~ s/\t/$lineprefix/g;
> +    print $lineprefix . $args{'type'} . " " . $args{'struct'} . " {\n$declaration" . $lineprefix . "};\n\n";
>  
> -    print "**Members**\n\n";
>      $lineprefix = "  ";
> +    print $lineprefix . "**Members**\n\n";
>      foreach $parameter (@{$args{'parameterlist'}}) {
>  	($parameter =~ /^#/) && next;
>  
> @@ -1003,8 +1013,10 @@ sub output_struct_rst(%) {
>  	($args{'parameterdescs'}{$parameter_name} ne $undescribed) || next;
>  	$type = $args{'parametertypes'}{$parameter};
>          print_lineno($parameterdesc_start_lines{$parameter_name});
> -	print "``" . $parameter . "``\n";
> +	print $lineprefix . "``" . $parameter . "``\n";
> +	$lineprefix = "    ";
>  	output_highlight_rst($args{'parameterdescs'}{$parameter_name});
> +	$lineprefix = "  ";
>  	print "\n";
>      }
>      print "\n";

-- 
Jani Nikula, Intel Open Source Graphics Center

Powered by blists - more mailing lists