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. Thanks, Mauro