On Tue, 19 Mar 2019 at 03:46, Jeff King <peff@xxxxxxxx> wrote: > > 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"> </refmiscinfo> > <refmiscinfo class="manual"> </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. > > 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. > > * 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)". > 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
