Messages in this thread | | | Date | Fri, 26 Apr 2019 13:37:19 -0600 | From | Jonathan Corbet <> | Subject | Re: [PATCH 1/2] Docs: An initial automarkup extension for sphinx |
| |
On Fri, 26 Apr 2019 15:32:55 -0300 Mauro Carvalho Chehab <mchehab+samsung@kernel.org> 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
| |