lkml.org 
[lkml]   [2019]   [Jun]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 04/14] ABI: better identificate tables
Em Wed, 19 Jun 2019 13:14:08 -0300
Mauro Carvalho Chehab <mchehab+samsung@kernel.org> escreveu:

> 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

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.

Thanks,
Mauro

\
 
 \ /
  Last update: 2019-06-20 16:28    [W:0.080 / U:0.340 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site