On Fri, 26 Apr 2019 15:32:55 -0300 Mauro Carvalho Chehab <mchehab+samsung@xxxxxxxxxx> wrote: > > +# Try to identify "function()" that's not already marked up some > > +# other way. Sphinx doesn't like a lot of stuff right after a > > +# :c:func: block (i.e. ":c:func:`mmap()`s" flakes out), so the last > > +# bit tries to restrict matches to things that won't create trouble. > > +# > > +RE_function = re.compile(r'(^|\s+)([\w\d_]+\(\))([.,/\s]|$)') > > IMHO, this looks good enough to avoid trouble, maybe except if one > wants to write a document explaining this functionality at the > doc-guide/kernel-doc.rst. Adding something to the docs is definitely on my list. > Anyway, the way it is written, we could still explain it by adding > a "\ " after the func, e. g.: > > When you write a function like: func()\ , the automarkup > extension will automatically convert it into: > ``:c:func:`func()```. > > So, this looks OK on my eyes. Not sure I like that; the whole point is to avoid extra markup here. Plus I like that it catches all function references whether the author thought to mark them or not. > > +# > > +# Lines consisting of a single underline character. > > +# > > +RE_underline = re.compile(r'^([-=~])\1+$') > > Hmm... why are you calling this "underline"? Sounds a bad name to me, > as it took me a while to understand what you meant. Seemed OK to me, but I can change it :) > From the code I'm inferring that this is meant to track 3 of the > possible symbols used as a (sub).*title markup. On several places > we use other symbols:'^', '~', '.', '*' (and others) as sub-sub(sub..) > title markups. I picked the ones that were suggested in our docs; it was enough to catch all of the problems in the current kernel docs. Anyway, The real documentation gives the actual set, so I'll maybe make it: =-'`":~^_*+#<> I'd prefer that to something more wildcardish. > You should probably need another regex for the title itself: > > RE_possible_title = re.compile(r'^(\S.*\S)\s*$') > > in order to get the size of the matched line. Doing a doing len(previous) > will get you false positives. This I don't quite get. It's easy enough to trim off the spaces with strip() if that turns out to be a problem (which it hasn't so far). I can add that. > on a separate matter (but related to automarkup matter - and to what > I would name underline), as a future feature, perhaps we could also add > a parser for: > > _something that requires underlines_ > > Underlined text is probably the only feature that we use on several docs > with Sphinx doesn't support (there are some extensions for that - I guess, > but it sounds simple enough to have a parser here). > > This can be tricky to get it right, as just underlines_ is a > cross reference markup - so, I would only add this after we improve the > script to come after Sphinx own markup processing. That does indeed sound tricky. It would also probably have to come *before* Sphinx does its thing or it's unlikely to survive. > > + # > > + # Is this an underline line? If so, and it is the same length > > + # as the previous line, we may have mangled a heading line in > > + # error, so undo it. > > + # > > + elif RE_underline.match(line): > > + if len(line) == len(previous): > > No, that doesn't seem enough. I would, instead, use the regex I > proposed before, in order to check if the previous line starts with > a non-space, and getting the length only up to the last non-space > (yeah, unfortunately, we have some text files that have extra blank > spaces at line's tail). So I'll make it "if len(line) == len(previous.strip()) Thanks, jon