[PATCH 0/2] Documentation: sphinx: Add sphinx-prompt

[PATCH 0/2] Documentation: sphinx: Add sphinx-prompt

[Nishanth Menon]

Hi,
Discussion started off in u-boot documentation[1] and Heinrich
suggested[2] getting something similar done for kernel as well.

https://youtu.be/ItjdVa59jjE shows how this change helps.

I have picked just a trivial file to show the impact, but it can
probably get done for other files as well.

NOTE: I am a sphinx noob, so, there might be better approaches.
Suggestions welcome.

Nishanth Menon (2):
  Documentation: sphinx: Add sphinx-prompt
  Documentation: bpf: Use sphinx-prompt

 Documentation/bpf/libbpf/libbpf_build.rst | 20 ++++++++++----------
 Documentation/conf.py                     |  2 +-
 Documentation/sphinx/requirements.txt     |  1 +
 3 files changed, 12 insertions(+), 11 deletions(-)

[1] https://lore.kernel.org/all/87fs48rgto.fsf@baylibre.com/
[2] https://lore.kernel.org/r/Datecc5d8c56-546a-4cd7-6a78-1206e9675555@canonical.com
-- 
2.40.0

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

[Nishanth Menon]

Sphinx-prompt[1] helps bring-in '.. prompt::' option that allows a
better rendered documentation, yet be able to copy paste without
picking up the prompt from the rendered documentation.

[1] https://pypi.org/project/sphinx-prompt/
Link: https://lore.kernel.org/all/87fs48rgto.fsf@baylibre.com/
Suggested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Suggested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
I would have added Reported-by for Simon, since he reported the issue in
the first place.. but it was for the u-boot documentation, so skipping
here.

 Documentation/conf.py                 | 2 +-
 Documentation/sphinx/requirements.txt | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/conf.py b/Documentation/conf.py
index d4fdf6a3875a..2eff713c4728 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -55,7 +55,7 @@ needs_sphinx = '1.7'
 extensions = ['kerneldoc', 'rstFlatTable', 'kernel_include',
               'kfigure', 'sphinx.ext.ifconfig', 'automarkup',
               'maintainers_include', 'sphinx.ext.autosectionlabel',
-              'kernel_abi', 'kernel_feat']
+              'kernel_abi', 'kernel_feat', 'sphinx-prompt']
 
 if major >= 3:
     if (major > 3) or (minor > 0 or patch >= 2):
diff --git a/Documentation/sphinx/requirements.txt b/Documentation/sphinx/requirements.txt
index 335b53df35e2..24a59ceda582 100644
--- a/Documentation/sphinx/requirements.txt
+++ b/Documentation/sphinx/requirements.txt
@@ -1,3 +1,4 @@
 # jinja2>=3.1 is not compatible with Sphinx<4.0
 jinja2<3.1
 Sphinx==2.4.4
+sphinx-prompt==1.5.0
-- 
2.40.0

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

[Daniel Borkmann]

On 8/24/23 8:21 PM, Nishanth Menon wrote:
> Sphinx-prompt[1] helps bring-in '.. prompt::' option that allows a
> better rendered documentation, yet be able to copy paste without
> picking up the prompt from the rendered documentation.
> 
> [1] https://pypi.org/project/sphinx-prompt/
> Link: https://lore.kernel.org/all/87fs48rgto.fsf@baylibre.com/
> Suggested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Suggested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>

Given the patch 2/2 is targeted for bpf docs, we can route this via bpf-next.
Jonathan, could we get an ack for this one if it looks good to you?

Thanks,
Daniel

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

[Jonathan Corbet]

Nishanth Menon <nm@ti.com> writes:

> Sphinx-prompt[1] helps bring-in '.. prompt::' option that allows a
> better rendered documentation, yet be able to copy paste without
> picking up the prompt from the rendered documentation.
>
> [1] https://pypi.org/project/sphinx-prompt/
> Link: https://lore.kernel.org/all/87fs48rgto.fsf@baylibre.com/
> Suggested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> Suggested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
> I would have added Reported-by for Simon, since he reported the issue in
> the first place.. but it was for the u-boot documentation, so skipping
> here.
>
>  Documentation/conf.py                 | 2 +-
>  Documentation/sphinx/requirements.txt | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)

So it would sure be nice for the changelog to say what this actually
does.

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.

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
benefit is sufficient, but I'm pretty far from convinced that this is
the case here.  Certainly the case hasn't been made in the changelog.
What *is* the benefit of making this change?

Thanks,

jon

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

[Nishanth Menon]

Hi Jon,

On 16:46-20230825, Jonathan Corbet wrote:
> Nishanth Menon <nm@ti.com> writes:
> 
> > Sphinx-prompt[1] helps bring-in '.. prompt::' option that allows a
> > better rendered documentation, yet be able to copy paste without
> > picking up the prompt from the rendered documentation.
> >
> > [1] https://pypi.org/project/sphinx-prompt/
> > Link: https://lore.kernel.org/all/87fs48rgto.fsf@baylibre.com/
> > Suggested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> > Suggested-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > Signed-off-by: Nishanth Menon <nm@ti.com>
> > ---
> > I would have added Reported-by for Simon, since he reported the issue in
> > the first place.. but it was for the u-boot documentation, so skipping
> > here.
> >
> >  Documentation/conf.py                 | 2 +-
> >  Documentation/sphinx/requirements.txt | 1 +
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> 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.

If you could recommend changes, I'd be glad to incorporate the same.

> 
> 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?

> 
> 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@ti.com/

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.

> benefit is sufficient, but I'm pretty far from convinced that this is
> the case here.  Certainly the case hasn't been made in the changelog.
> What *is* the benefit of making this change?

Let me know *how* I can improve (note: I am not a native English
speaker, so, I'd appreciate any suggestions to make the argument clear
in the changelog). Intent here is to help make the rendered html
documentation that we publish in docs.kernel.org such as
https://docs.kernel.org/bpf/libbpf/libbpf_build.html better usable.


-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

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

[Jonathan Corbet]

Nishanth Menon <nm@ti.com> 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@ti.com/
>
> 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

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

[Nishanth Menon]

On 07:41-20230828, Jonathan Corbet wrote:
> Nishanth Menon <nm@ti.com> 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.

True, and fair enough.

> 
> >> 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@ti.com/
> >
> > 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.

It is no additional cognitive load from what is already there:

-.. code-block:: bash
+.. prompt:: bash $

> 
> 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?

Just hated people who read online documentation from having to spend
extra few seconds in copy pasting and then realizing oops "$" came along
with it.

> 
> 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.

:) OK - I tried.. Thanks for explaining (though I disagree).
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

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

[Matthew Wilcox]

On Mon, Aug 28, 2023 at 07:41:39AM -0600, Jonathan Corbet wrote:
> 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.

I agree that the way this has been presented is awful.

> > 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.

I'd ask that you reconsider.  Looking at patch 2, I prefer what is
written there.  I don't think it adds cognitive load when reading the
plain docs.  I find the "copy and paste from html" argument not very
convincing, but I do like "copy and paste from rst", which this enables.

I also have a certain fond memory of how the plan9 people set up 'rc'
(their shell) so that ";" was both an empty statement, and the default
prompt.  So you could copy-paste lines starting with the ; prompt and
they'd work.  It's a small usabillity improvement, but it is there,
and wow is it annoying when you don't have it any more.

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

[Jonathan Corbet]

Matthew Wilcox <willy@infradead.org> writes:

> On Mon, Aug 28, 2023 at 07:41:39AM -0600, Jonathan Corbet wrote:
>> 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.
>
> I'd ask that you reconsider.  Looking at patch 2, I prefer what is
> written there.  I don't think it adds cognitive load when reading the
> plain docs.  I find the "copy and paste from html" argument not very
> convincing, but I do like "copy and paste from rst", which this enables.

Do you really think that the benefit from that justifies adding a build
dependency and breaking everybody's docs build until they install it?  I
rather suspect I would hear back from people who feel otherwise if I did
that... 

> I also have a certain fond memory of how the plan9 people set up 'rc'
> (their shell) so that ";" was both an empty statement, and the default
> prompt.  So you could copy-paste lines starting with the ; prompt and
> they'd work.  It's a small usabillity improvement, but it is there,
> and wow is it annoying when you don't have it any more.

Ah, OK, so what we really need is a bash patch :)

Thanks,

jon

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

[Mauro Carvalho Chehab]

Em Mon, 28 Aug 2023 09:12:07 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:

> Matthew Wilcox <willy@infradead.org> writes:
> 
> > On Mon, Aug 28, 2023 at 07:41:39AM -0600, Jonathan Corbet wrote:  
> >> 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.  
> >
> > I'd ask that you reconsider.  Looking at patch 2, I prefer what is
> > written there.  I don't think it adds cognitive load when reading the
> > plain docs.  I find the "copy and paste from html" argument not very
> > convincing, but I do like "copy and paste from rst", which this enables.  
> 
> Do you really think that the benefit from that justifies adding a build
> dependency and breaking everybody's docs build until they install it?  I
> rather suspect I would hear back from people who feel otherwise if I did
> that... 

I agree with Jon: it needs at least a patch for scripts/sphinx-pre-install.
Adding dependencies there is not the easiest thing to do, as one needs to
test the change against all supported distros to ensure that the new package
name will be the same everywhere. Also, if I'm not mistaken, some developers
don't want to use pip to install packages, wanting instead to have the
distro-provided package.

Also, having an extra build dependency will surely break already-existing
CI automation. Making the new dependency optional would be a way to go,
but this will cause troubles at the html output after such change.

> > I also have a certain fond memory of how the plan9 people set up 'rc'
> > (their shell) so that ";" was both an empty statement, and the default
> > prompt.  So you could copy-paste lines starting with the ; prompt and
> > they'd work.  It's a small usabillity improvement, but it is there,
> > and wow is it annoying when you don't have it any more.  
> 
> Ah, OK, so what we really need is a bash patch :)

Probably the hardest part would be to do copy-and-paste on places
where there are both shell prompt commands and their results. I'm
pretty sure we have things like:

	some example::

		$ run_some_command
		comand results line 1
		comand results line 2
		comand results line 3
		...
		comand results line n

		$ run_another_command

does sphinx-prompt handle things like that, placing just:

	run_some_command
	run_another_command

at the paste buffer, ignoring any command result lines?

IMO, the above described usease is where having a prompt will help
to identify what should be copied/pasted and what are the command
results. I mean, if someone wants to just place the commands to
run, he could write, instead:

	Run those shell commands to do something::

		run_some_command
		run_another_command


Regards,
Mauro

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

[Jonathan Corbet]

Mauro Carvalho Chehab <mchehab@kernel.org> writes:

> Adding dependencies there is not the easiest thing to do, as one needs to
> test the change against all supported distros to ensure that the new package
> name will be the same everywhere. Also, if I'm not mistaken, some developers
> don't want to use pip to install packages, wanting instead to have the
> distro-provided package.

That, actually, is something we definitely need to keep in mind.  The
security record for PyPI (as with almost all of the language-specific
repos) is not great.  We need to think pretty hard before telling
developers (or, say, the build process on kernel.org) that they need to
install packages from there on their systems.

Thanks,

jon

[PATCH 2/2] Documentation: bpf: Use sphinx-prompt

[Nishanth Menon]

Use Sphinx-prompt to generate a better rendered documentation that is
easier for copy-paste.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 Documentation/bpf/libbpf/libbpf_build.rst | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/Documentation/bpf/libbpf/libbpf_build.rst b/Documentation/bpf/libbpf/libbpf_build.rst
index 8e8c23e8093d..2b94e5778702 100644
--- a/Documentation/bpf/libbpf/libbpf_build.rst
+++ b/Documentation/bpf/libbpf/libbpf_build.rst
@@ -13,25 +13,25 @@ setting NO_PKG_CONFIG=1 when calling make.
 
 To build both static libbpf.a and shared libbpf.so:
 
-.. code-block:: bash
+.. prompt:: bash $
 
-    $ cd src
-    $ make
+    cd src
+    make
 
 To build only static libbpf.a library in directory build/ and install them
 together with libbpf headers in a staging directory root/:
 
-.. code-block:: bash
+.. prompt:: bash $
 
-    $ cd src
-    $ mkdir build root
-    $ BUILD_STATIC_ONLY=y OBJDIR=build DESTDIR=root make install
+    cd src
+    mkdir build root
+    BUILD_STATIC_ONLY=y OBJDIR=build DESTDIR=root make install
 
 To build both static libbpf.a and shared libbpf.so against a custom libelf
 dependency installed in /build/root/ and install them together with libbpf
 headers in a build directory /build/root/:
 
-.. code-block:: bash
+.. prompt:: bash $
 
-    $ cd src
-    $ PKG_CONFIG_PATH=/build/root/lib64/pkgconfig DESTDIR=/build/root make
\ No newline at end of file
+    cd src
+    PKG_CONFIG_PATH=/build/root/lib64/pkgconfig DESTDIR=/build/root make
-- 
2.40.0