Message-ID: <874kgtq079.fsf@intel.com>
Date:   Mon, 29 Mar 2021 21:33:30 +0300
From:   Jani Nikula <jani.nikula@...ux.intel.com>
To:     Matthew Wilcox <willy@...radead.org>,
        Jonathan Corbet <corbet@....net>
Cc:     Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>,
        Rob Herring <robh@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kernel-doc: better handle '::' sequences

On Mon, 29 Mar 2021, Matthew Wilcox <willy@...radead.org> wrote:
> On Thu, Mar 25, 2021 at 04:30:32PM -0600, Jonathan Corbet wrote:
>> Matthew Wilcox <willy@...radead.org> writes:
>> 
>> > The rust code is alredy coming though ...
>> >
>> > rust/kernel/buffer.rs:/// A pre-allocated buffer that implements [`core::fmt::Write`].
>> >
>> > so now we have three formats.  Markdown and RST are _very_ similar, but
>> > not identical [1].  Oh, and even better we now have three distinct tools --
>> > kerneldoc, rustdoc and sphinx.  Have the rust people reached out to you
>> > about integrating the various docs?
>> 
>> I have talked with them a bit, yes, but without any clear conclusions at
>> this point.  The Rust world has its own way of doing things with regard
>> to documentation, and I don't want to tell them they can't use it in the
>> kernel context.  So I think there's going to be a certain amount of
>> groping around for the best solution.
>> 
>> We did come to the mutual agreement that teaching kernel-doc to parse
>> Rust code as well was not an ideal solution.  Probably there will be
>> some sort of tool to translate between rustdoc and our sphinx setup.
>> Beyond that, we'll see how it goes.
>
> In the spirit of groping around for the best solution, I did some looking
> around at various options, including using rustdoc for .c files (that
> uses Markdown, which appears to be strictly worse than rST for our
> purposes).
>
> So here's my "modest proposal":
>
>  - Similar to our ".. kernel-doc::" invocation in .rst files, handle
>    ".. rustdoc::" (insert weeks of hacking here)
>  - Now add ".. rst-doc::" which parses .c files like [1] kernel-doc
>    does, but interprets a different style of comment and actually does
>    most of the repetitive boring bits for you.

As a hobby, I've written a Sphinx extension to use Clang to parse the
code and extract pure reStructuredText documentation comments with
minimal conversions [1]. No additional syntax. Just use reStructuredText
for everything instead of inventing your own.

I'm not proposing to use that in kernel, at all. It was more like a
diversion from the kernel documentation.

But based on my experience with the old and new kernel documentation
systems and the hobby one, the one takeaway is to not create new
syntaxes, grammars, parsers, or preprocessors to be maintained by the
kernel community. Just don't. Take what's working and supported by other
projects, and add the minimal glue using Sphinx extensions to put it
together, and no more.

Of course, we couldn't ditch kernel-doc the script, but we managed to
trim it down quite a bit. OTOH, there have been a number of additions
outside of Sphinx in Makefiles and custom tools in various languages
that I'm really not happy about. It's all too reminiscient of the old
DocBook toolchain, while Sphinx was supposed to be the one tool to tie
it all together, partially chosen because of the extension support.


BR,
Jani.


[1] https://github.com/jnikula/hawkmoth


>
> For example, xa_load:
>
> /**
>  * xa_load() - Load an entry from an XArray.
>  * @xa: XArray.
>  * @index: index into array.
>  *
>  * Context: Any context.  Takes and releases the RCU lock.
>  * Return: The entry at @index in @xa.
>  */
> void *xa_load(struct xarray *xa, unsigned long index)
>
> //rST
> // Load an entry from an XArray.
> //
> // :Context: Any context.  Takes and releases the RCU lock.
> // :Return: The entry in `xa` at `index`.
> void *xa_load(struct xarray *xa, unsigned long index)
>
> (more complex example below [2])
>
> Things I considered:
>
>  - Explicitly document that this is rST markup instead of Markdown or
>    whatever.
>  - Don't repeat the name of the function.  The tool can figure it out.
>  - Don't force documenting each parameter.  Often they are obvious
>    and there's really nothing interesting to say about the parameter.
>    Witness the number of '@foo: The foo' (of type struct foo) that we
>    have scattered throughout the tree.  It's not that the documenter is
>    lazy, it's that there's genuinely nothing to say here.
>  - Use `interpreted text` to refer to parameters instead of *emphasis* or
>    **strong emphasis**.  The tool can turn that into whatever markup
>    is appropriate.
>  - Use field lists for Context and Return instead of sections.  The markup
>    is simpler to use, and I think the rendered output is better.
>
> [1] by which i mean "in a completely different way from, but similar in
>     concept"
>
> [2] More complex example:
>
> /**
>  * xa_store() - Store this entry in the XArray.
>  * @xa: XArray.
>  * @index: Index into array.
>  * @entry: New entry.
>  * @gfp: Memory allocation flags.
>  *
>  * After this function returns, loads from this index will return @entry.
>  * Storing into an existing multi-index entry updates the entry of every index.
>  * The marks associated with @index are unaffected unless @entry is %NULL.
>  *
>  * Context: Any context.  Takes and releases the xa_lock.
>  * May sleep if the @gfp flags permit.
>  * Return: The old entry at this index on success, xa_err(-EINVAL) if @entry
>  * cannot be stored in an XArray, or xa_err(-ENOMEM) if memory allocation
>  * failed.
>  */
> void *xa_store(struct xarray *xa, unsigned long index, void *entry, gfp_t gfp)
>
> //rST
> // Store an entry in the XArray.
> //
> // After this function returns, loads from `index` will return `entry`.
> // Storing into an existing multi-index entry updates the entry of every index.
> // The marks associated with `index` are unaffected unless `entry` is ``NULL``.
> //
> // :Context: Any context.  Takes and releases the xa_lock.
> //    May sleep if the `gfp` flags permit.
> // :Return: The old entry at this index on success, xa_err(-EINVAL) if `entry`
> //    cannot be stored in an XArray, or xa_err(-ENOMEM) if memory allocation
> //    failed.
> void *xa_store(struct xarray *xa, unsigned long index, void *entry, gfp_t gfp)
>

-- 
Jani Nikula, Intel Open Source Graphics Center

Powered by blists - more mailing lists