Re: [PATCH] asciidoctor-extensions: provide `<refmiscinfo/>`

From: Martin Ågren <martin.agren@gmail.com>
Date: Mar 19, 2019, 07:10 UTC
Message-ID: <CAN0heSoS_M-nYCTaL0jKBot8PWv1k-nscD9tgf3pgAz-Ddn-7g@mail.gmail.com>
In-Reply-To: <20190319024645.GA6173@sigill.intra.peff.net>

On Tue, 19 Mar 2019 at 03:46, Jeff King <peff@peff.net> wrote:

Show 24 quoted lines

>
> On Sun, Mar 17, 2019 at 03:47:47PM +0100, Martin Ågren wrote:
>
> >  Cc Todd and Peff who had a brief exchange [1] a while ago. Apparently
> >  Todd saw this "[FIXME: source]" on Fedora but Peff did not on Debian.
> >  I'm on Ubuntu 18.04 and used to see this also on 16.04. I'm not sure
> >  what might make Debian so special here.
>
> I think it was just that my version of asciidoctor had
>
>   https://github.com/asciidoctor/asciidoctor/pull/2636
>
> and Todd's did not. However, mine still does not do the _right_ thing,
> because we didn't pass the right attributes in to asciidoctor. It just
> didn't print an ugly "FIXME". Looking at the XML, I have:
>
>   <refentrytitle>git-add</refentrytitle>
>   <manvolnum>1</manvolnum>
>   <refmiscinfo class="source">&#160;</refmiscinfo>
>   <refmiscinfo class="manual">&#160;</refmiscinfo>
>   </refmeta>
>
> So it's just an nbsp instead of the real content, and the "version"
> field is missing entirely.

Huh, yeah, that's a big improvement already.

Show 21 quoted lines

> > That Asciidoctor ignores asciidoc.conf is nothing new. This is why we
> > implement the `linkgit:` macro in asciidoc.conf *and* in
> > asciidoctor-extensions.rb. Follow suit and provide these tags in
> > asciidoctor-extensions.rb, using a "postprocessor" extension.
>
> Yeah, that seems sensible overall. Some thoughts on your approach:
>
> >   * Provide the `mansource` attribute to Asciidoctor. This attribute
> >     looks promising until one realizes that it can only be given inside
> >     the source file (the .txt file in our case), *not* on the command
> >     line using `-a mansource=foobar`. I toyed with the idea of injecting
> >     this attribute while feeding Asciidoctor the input on stdin, but it
> >     didn't feel like it was worth the complexity in the Makefile.
>
> It does seem like "mansource" is the way asciidoctor expects us to do
> this. Why doesn't it work from the command line? Is it a bug in
> asciidoctor, or is there something more subtle going on?
>
> I think even if it is a bug and gets fixed, though, it still wouldn't
> have the version field (though that seems like something we could
> contribute to asciidoctor).

The bug is in my docs-reading, see below.

Show 15 quoted lines

> >   * Considering the above abandoned ideas, it seems better to put any
> >     complexity inside asciidoctor-extensions.rb. It is after all
> >     supposed to be the "equivalent" of asciidoc.conf. I considered
> >     providing a "tree processor" extension and use it to set, e.g.,
> >     `mansource` mentioned above.
>
> This seems like the least bad option, at least for now. Your code does
> do a generic regex substitution. The promise of XML is that we're
> supposed to be able to do structured, robust transformations of the
> document. But my experience has been that the tooling is sufficiently
> difficult to work with that you just end up writing a regex.
>
> So I'm curious if you tried to use an actual XML parser (or god forbid,
> XSLT) to do the transformation. But if you spent more than 5 minutes on
> it and got disgusted, I wouldn't ask you to look deeper than that. :)

Well, I didn't spend 5 minutes on it, but my experience told me something like that would happen. ;-)

Now I realize that I'm wrong in my "it doesn't work from the command line". Somehow, I read the following in the user manual: "Many attributes can only be defined in the document header (or via the API or CLI)." And *repeatedly* read is as "(not via the API or CLI)", somehow always expecting a "not" to follow an "only". Oh well.

But of course, I did try it out before reaching for the docs, like anyone would. The true reason it doesn't work for me is probably this header from the listing that contains "mansource": "Manpage attributes (relevant only when using the manpage doctype and/or converter)".

Show 14 quoted lines

> I doubt we'd see any other refmeta tags (and any non-tag content would
> be quoted).
>
> > Let's instead try to stay as close as possible to what asciidoc.conf
> > does. We'll make it fairly obvious that we aim to inject the exact same
> > three lines of `<refmiscinfo/>` that asciidoc.conf provides. The only
> > somewhat tricky part is that we inject them *post*-processing so we need
> > to do the variable expansion ourselves.
>
> One thing that asciidoctor buys us that asciidoc does not is that we
> might eventually move to directly generating the manpages, without the
> XML / Docbook step in between. And if we do, then all of this XML
> hackery is going to have to get replaced with something else. I guess we
> can cross that bridge when we come to it.

Todd has made a promising start in another part of this thread. There seems to be a few wrinkles that need some care, but hopefully nothing impossible (famous last words).

> The patch itself looks sane. Would we ever need to XML-quote the
> contents of git_version? I guess the asciidoc.conf version doesn't
> bother.

Good point. Hadn't thought of it. You're right that the asciidoc.conf version has the same problem and a version string like "<>" goes unescaped into the xml.

> Technically the user running "make" could put whatever they want
> into it, but I think this is a case of "if it hurts, don't do it", and
> we can ignore it.

:-)

Martin