Re: [PATCH 1/2] Documentation: sphinx: Add sphinx-prompt

From: Jonathan Corbet
Date: Mon Aug 28 2023 - 09:43:16 EST


Nishanth Menon <nm@xxxxxx> writes:

> Hi Jon,
>
> On 16:46-20230825, Jonathan Corbet wrote:

>> So it would sure be nice for the changelog to say what this actually
>> does.
>
> All this does is to bring in a better rendered documentation when
> published in html format.
> https://youtu.be/ItjdVa59jjE shows how the "copy-paste" functionality is
> improved.

Youtube references aren't a great way to explain the value of a patch;
you'll find that maintainers will, in general, lack the time or
inclination to follow them up. The patch should explain itself.

>> This appears to add a build dependency for the docs; we can't just add
>> that without updating the documentation, adjusting
>> scripts/sphinx-pre-install, and so on.
>
> I had checked scripts/shinx-pre-install and that picks up
> Documentation/sphinx/requirements.txt and installs the dependencies
> from there using pip. Am I missing something?
>
> Same thing with Documentation/doc-guide/sphinx.rst
>
> Am I missing something?

That works, I guess, but doesn't change the fact that you have added
another docs build dependency. That will, among other things, break the
build for anybody who is set up to do it now until they install your new
package. That's not something we want to do without good reason.

>> But, beyond that, this extension goes entirely counter to the idea that
>> the plain-text files are the primary form of documentation; it adds
>> clutter and makes those files less readable. We can do that when the
>
> Are you sure this is going against the readable text documentation? If
> anything it reduces the clutter and allows the text doc to be
> copy-paste-able as well.
>
> https://lore.kernel.org/all/20230824182107.3702766-3-nm@xxxxxx/
>
> As you see from the diffstat:
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> Nothing extra added. What kind of clutter are you suggesting we added
> with the change?
>
> prompt:: bash $ is clearly readable that this is prompt documentation
> in fact, dropping the "$" in the example logs, one can easily copy paste
> the documentation from rst files as well.

.. prompt:: is clutter. It also adds a bit of extra cognitive load to
reading that part of the documentation.

Quick copy-paste of multiple lines of privileged shell commands has
never really been a requirement for the kernel docs; why do we need that
so badly?

I appreciate attempts to improve our documentation, and hope that you
will continue to do so. I am far from convinced, though, that this
change clears the bar for mainline inclusion.

Thanks,

jon