On Thu, Jun 20, 2019 at 11:27:49AM -0300, Mauro Carvalho Chehab wrote:

Em Wed, 19 Jun 2019 13:14:08 -0300
Mauro Carvalho Chehab [off-list ref] escreveu:

quoted

Em Wed, 19 Jun 2019 17:02:07 +0200
Greg Kroah-Hartman [off-list ref] escreveu:

quoted

On Wed, Jun 19, 2019 at 10:56:33AM -0300, Mauro Carvalho Chehab wrote:  

quoted

Hi Johan,

Em Wed, 19 Jun 2019 14:51:35 +0200
Johan Hovold [off-list ref] escreveu:
    

quoted

On Thu, Jun 13, 2019 at 11:04:10PM -0300, Mauro Carvalho Chehab wrote:    

quoted

From: Mauro Carvalho Chehab <redacted>

When parsing via script, it is important to know if the script
should consider a description as a literal block that should
be displayed as-is, or if the description can be considered
as a normal text.

Change descriptions to ensure that the preceding line of a table
ends with a colon. That makes easy to identify the need of a
literal block.      

In the cover letter you say that the first four patches of this series,
including this one, "fix some ABI descriptions that are violating the
syntax described at Documentation/ABI/README". This seems a bit harsh,
given that it's you that is now *introducing* a new syntax requirement
to assist your script.    

Yeah, what's there at the cover letter doesn't apply to this specific
patch. The thing is that I wrote this series a lot of time ago (2016/17).

I revived those per a request at KS ML, as we still need to expose the
ABI content on some book that will be used by userspace people.

So, I just rebased it on the top of curent Kernel, add a cover letter
with the things I remembered and re-sent.

In the specific case of this patch, the ":" there actually makes sense
for someone that it is reading it as a text file, and it is an easy
hack to make it parse better.
    

quoted

Specifically, this new requirement isn't documented anywhere AFAICT, so
how will anyone adding new ABI descriptions learn about it?    

Yeah, either that or provide an alternative to "Description" tag, to be
used with more complex ABI descriptions.

One of the things that occurred to me, back on 2017, is that we should
have a way to to specify that an specific ABI description would have
a rich format. Something like:

What:		/sys/bus/usb/devices/<busnum>-<devnum>:<config num>.<interface num>/<hid-bus>:<vendor-id>:<product-id>.<num>/pyra/roccatpyra<minor>/actual_cpi
Date:		August 2010
Contact:	Stefan Achatz [off-list ref]
RST-Description:
		It is possible to switch the cpi setting of the mouse with the
		press of a button.
		When read, this file returns the raw number of the actual cpi
		setting reported by the mouse. This number has to be further
		processed to receive the real dpi value:

		===== =====
		VALUE DPI
		===== =====
		1     400
		2     800
		4     1600
		===== =====

With that, the script will know that the description contents will be using
the ReST markup, and parse it accordingly. Right now, what it does, instead,
is to place the description on a code-block, e. g. it will produce this
output for the description:

::

		It is possible to switch the cpi setting of the mouse with the
		press of a button.
		When read, this file returns the raw number of the actual cpi
		setting reported by the mouse. This number has to be further
		processed to receive the real dpi value:

		VALUE DPI
		1     400
		2     800
		4     1600


Greg, 

what do you think?    

I don't know when "Description" and "RST-Description" would be used.
Why not just parse "Description" like rst text and if things are "messy"
we fix them up as found, like you did with the ":" here?  It doesn't
have to be complex, we can always fix them up after-the-fact if new
stuff gets added that doesn't quite parse properly.

Just like we do for most kernel-doc formatting :)  

Works for me. Yet, I guess I tried that, back on 2017. 

If I'm not mistaken, the initial patchset to solve the broken things 
won't be small, and will be require a lot of attention in order to
identify what's broken and where.

Btw, one thing is to pass at ReST validation. Another thing is to
produce something that people can read. 

Right now, the pertinent logic at the script I wrote (scripts/get_abi.pl)
is here:

                if (!($desc =~ /^\s*$/)) {
                        if ($desc =~ m/\:\n/ || $desc =~ m/\n[\t ]+/  || $desc =~ m/[\x00-\x08\x0b-\x1f\x7b-\xff]/) {
                                # put everything inside a code block
                                $desc =~ s/\n/\n /g;

                                print "::\n\n";
                                print " $desc\n\n";
                        } else {
                                # Escape any special chars from description
                                $desc =~s/([\x00-\x08\x0b-\x1f\x21-\x2a\x2d\x2f\x3c-\x40\x5c\x5e-\x60\x7b-\xff])/\\$1/g;

                                print "$desc\n\n";
                        }
                }

If it discovers something weird enough, it just places everything
into a comment block. Otherwise, it assumes that it is a plain
text and that any special characters should be escaped.

If the above block is replaced by a simple:

		print "$desc\n\n";

The description content will be handled as a ReST file.

I don't have any time right now to do this change and to handle the
warnings that will start to popup.

Btw, a single replace there is enough to show the amount of problems that
it will rise, as it will basically break Sphinx build with:

	reading sources... [  1%] admin-guide/abi-testing
	reST markup error:
	get_abi.pl rest --dir $srctree/Documentation/ABI/testing:45261: (SEVERE/4) Missing matching underline for section title overline.
	
	==========================
	PCIe Device AER statistics
	These attributes show up under all the devices that are AER capable. These

To be clear here: the problem with the above is that ReST has zero
tolerance and actually behaves like a crash, if it receives something like:

   ==========================
   PCIe Device AER statistics
   ==========================

For it to work, it has to have zero spaces before ===..=== line, e. g.:

==========================
PCIe Device AER statistics
==========================

Ok, maybe we could try to teach the parser a way to identify the initial
spacing of the first description line and remove that amount of 
spaces/tabs for the following lines, but it may require some heuristics.

Or we can clean this type of thing up by hand :)

Let's see how bad this gets, the documentation in these files should not
be very complex as they _should_ only be one-value-per-file, but as you
have shown in this very example, that rule is violated :(

thanks,

greg k-h