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 <mchehab+samsung@xxxxxxxxxx> escreveu:
>
> > Em Wed, 19 Jun 2019 17:02:07 +0200
> > Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxx> escreveu:
> > > >
> > > > > On Thu, Jun 13, 2019 at 11:04:10PM -0300, Mauro Carvalho Chehab wrote:
> > > > > > From: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>
> > > > > >
> > > > > > 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@xxxxxxxxxxxxxxxxxxxxx>
> > > > 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
