Re: [PATCH 04/14] ABI: better identificate tables

[Mauro Carvalho Chehab <mchehab+samsung@kernel.org>]

Em Wed, 19 Jun 2019 17:02:07 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:

> On Wed, Jun 19, 2019 at 10:56:33AM -0300, Mauro Carvalho Chehab wrote:
> > Hi Johan,
> > 
> > Em Wed, 19 Jun 2019 14:51:35 +0200
> > Johan Hovold <johan@kernel.org> escreveu:
> >   
> > > On Thu, Jun 13, 2019 at 11:04:10PM -0300, Mauro Carvalho Chehab wrote:  
> > > > From: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > > > 
> > > > 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.
> >   
> > > 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 <erazor_de@users.sourceforge.net>
> > 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

Thanks,
Mauro

diff --git a/scripts/get_abi.pl b/scripts/get_abi.pl
index 7bc619b6890c..e75f441afdcd 100755
--- a/scripts/get_abi.pl
+++ b/scripts/get_abi.pl
@@ -288,18 +288,18 @@ sub output_rest {
 		$desc =~ s/\n[\-\*\=\^\~]+\n/\n/g;
 
 		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;
+#			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 "::\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";
-			}
+#			}
 		} else {
 			print "DESCRIPTION MISSING for $what\n\n" if (!$data{$what}->{is_file});
 		}