Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly

From: Jeff King <peff@peff.net>
Date: May 12, 2021, 06:59 UTC
Message-ID: <YJt81neO7zsGz2ah@coredump.intra.peff.net>
In-Reply-To: <YJt1/DO1cXNTRNxK@coredump.intra.peff.net>

On Wed, May 12, 2021 at 02:30:20AM -0400, Jeff King wrote:

Show 13 quoted lines

> On Wed, May 12, 2021 at 02:22:06AM -0400, Jeff King wrote:
> 
> > With that change, plus a patch I'll send in a minute, it's easy to run
> > doc-diff on the result.
> 
> And here that is (note that if you don't apply the flags fix I showed,
> then doc-diff gets confused, because it expects back-to-back runs of
> "make" to handle the changed flags correctly).
> 
> Feel free to add it to your series if it helps (I had originally thought
> it would just be a one-off to look at the output, but there are enough
> changes, both correctness and style, that it may be useful to have this
> option around).

Adding in the "\e" to the extensions string fixes many problems. Just skimming over the remaining changes (from asciidoctor+xmlto to asciidoctor direct), here are some things I see:

Both of the xmlto pipelines seem to insert extra space before a literal. The direct version fixes that. E.g.:

  -           Files to add content from. Fileglobs (e.g.  *.c) can be given to
  [...]
  +           Files to add content from. Fileglobs (e.g. *.c) can be given to add

So that's good. But what you can't see in the doc-diff rendered version is that we've lost the bolding on literals (this is from the <pathspec> option in git-add.1).

Another is that the direct version is less willing to break linkgit targets across lines:

  -           explained for the configuration variable core.quotePath (see git-
  -           config(1)). See also --pathspec-file-nul and global
  +           explained for the configuration variable core.quotePath (see
  +           git-config(1)). See also --pathspec-file-nul and global

That seems fine to me, though it unfortunately does make the diff quite noisy.

We seem to have a problem with some escape codes. E.g.:

  -           of nothing). The other file, git-add--interactive.perl, has 403
  -           lines added and 35 lines deleted if you commit what is in the
  -           index, but working tree file has further modifications (one
  +           of nothing). The other file, git-add&#x2d;&#x2d;interactive.perl,
  +           has 403 lines added and 35 lines deleted if you commit what is in
  +           the index, but working tree file has further modifications (one

and:

  -           Added content is represented by lines beginning with "+". You can
  -           prevent staging any addition lines by deleting them.
  +           Added content is represented by lines beginning with "&#43;". You
  +           can prevent staging any addition lines by deleting them.

which is a pretty bad regression.

The trailer misses the version field:

  -Git omitted                       1970-01-01                        GIT-ADD(1)
  +Git                               1970-01-01                        GIT-ADD(1)

The "omitted" is part of doc-diff's attempt to reduce noise in the diff. But you can see that it's missing entirely in the direct version.

There are lots of whitespace changes for lists. They mostly seem fine either way. It also formats numbered lists differently:

  -            1. Delete the remote-tracking branches "todo", "html" and
  +           (1) Delete the remote-tracking branches "todo", "html" and
                  "man". The next fetch or pull will create them again
                  unless you configure them not to. See git-fetch(1).
  -            2. Delete the "test" branch even if the "master" branch (or
  +           (2) Delete the "test" branch even if the "master" branch (or
                  whichever branch is currently checked out) does not have
                  all commits from the test branch.

I prefer the original, but could live with the latter (IIRC, this is something that can be configured via asciidoctor, but I didn't dig).

This one is quite curious (from gitworkflows(7)):

  -       Example 1. Merge upwards
  +       Rule: Merge upwards

The source looks like this:

  .Merge upwards
  [caption="Rule: "]

So it looks like it takes the caption, rather than the phrase "example" that I guess is coming from deep within the bowls of docbook. Both asciidoc and asciidoctor produce the "Example" text when going through xmlto, but both produce "Rule" when generating HTML. So I imagine the latter was the intent, and this is a fix.

Links are a bit harder to read. E.g.:

   SEE ALSO
          git-check-ref-format(1), git-fetch(1), git-remote(1), “Understanding
  -       history: What is a branch?”[1] in the Git User’s Manual.
  +       history: What is <user-manual.html#what-is-a-branch> a branch?”" in the
  +       Git User’s Manual.

There may be more lurking, but it's hard to tell given how noisy the diff is.

-Peff