patch, 11 partsdoc: asciidoctor: direct man page creation and fixes

This patch series enables direct man page creation with asciidoctor, but in addition tries to minimize the difference with asciidoc+docbook.

I fixed as many issues as I could, and now the doc-diff looks very sensible. I could not find any major issues, however, some minor issues still remain.

On the other hand the asciidoc method has issues as well, so it's not clear which method is superior at this point; both have advantages and disadvantages.

At the very least the man pages with pure asciidoctor should be quite usable now.

This series builds on top of my cleanups [1]. It's probably not worth the effort to split the two.

[1] https://lore.kernel.org/git/20210514115631.503276-1-felipe.contreras@gmail.com

Felipe Contreras (11):
  doc: allow the user to provide ASCIIDOC_EXTRA
  doc: doc-diff: allow more than one flag
  doc: doc-diff: set docdate manually
  doc: use asciidoctor to build man pages directly
  doc: asciidoctor: add linkgit macros in man pages
  doc: join mansource and manversion
  doc: add man pages workaround for asciidoctor
  doc: asciidoctor: add hack for xrefs
  doc: asciidoctor: add hack to improve links
  doc: asciidoctor: add support for baseurl
  doc: asciidoctor: cleanup man page hack

 Documentation/Makefile                  | 24 ++++++++-----
 Documentation/asciidoctor-extensions.rb | 46 +++++++++++++++++++++++++
 Documentation/doc-diff                  |  8 ++---
 3 files changed, 65 insertions(+), 13 deletions(-)

Without `override` all additions will be ignored by make.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

On Fri, May 14, 2021 at 07:14:25AM -0500, Felipe Contreras wrote:

That's true of all of our Makefile variables. Is there a particular reason to give this one special treatment?

-Peff

On Sat, May 15, 2021 at 05:32:08AM -0400, Jeff King wrote:

To go into further detail: usually we distinguish variables we use internally from user-facing ones, and include the latter in the former. I see a later patch wants to start passing ASCIIDOC_EXTRA on the command-line, and we'd use two variables for that.

-Peff

Jeff King wrote:

Well, it's not exactly user-facing; it's only needed for doc-diff.

Would TEST_ASCIIDOC_EXTRA make sense?

On Sat, May 15, 2021 at 07:13:48AM -0500, Felipe Contreras wrote:

It's meant for the caller of "make". Your proposed use is within doc-diff, but any user running "make ASCIIDOC_EXTRA=foo" would see the different behavior.

I'd probably call it ASCIIDOC_FLAGS (like we have CFLAGS and LDFLAGS that are meant for users to inform us of extra flags they'd like passed).

Of course that may not solve your problem in a sense; if you want doc-diff to override it, then that might conflict with a theoretical ASCIIDOC_FLAGS somebody set in their config.mak (but we really are in the realm of hypothetical here).

-Peff

Jeff King wrote:

Yeah, they would, but I don't think it would be wrong behavior.

Right, but Makefiles do override those, like:

  override CFLAGS += -fPIC

Otherwise builds may fail.

Setting ASCIIDOC_FLAGS in config.mk would not override the user-supplied flags any more than setting them in the Makefile (they are virtually the same thing as one includes the other).

It's only if the user has `override ASCIIDOC_FLAGS` in config.mk that such a problem would arise. And that's really hypothetical.

Cheers.

On Mon, May 17, 2021 at 05:53:25AM -0500, Felipe Contreras wrote:

It depends what they're trying to do. If they write:

  make ASCIIDOC_EXTRA=--one-extra-option

then they probably intend to to add to the options we set. If they write:

  make ASCIIDOC_EXTRA='-acompat-mode -atabsize=4 ...etc...'

with the intent of replicating the flags but changing or removing some elements, then it would no longer do what they want.

I don't mean to imply one is more right than the other (I'd suspect even that the override behavior is more likely to be what somebody wants). I'm mostly pointing out that this is unlike the rest of our Makefiles, which do not ever use override (and that the effect is visible to the caller, depending on what they want to do).

Some Makefiles do, but in this project we have not historically used override. Instead, we provide defaults for things like CFLAGS, expect the use to replace them if they like, and then aggregate them (along with other internal variables) into things like ALL_CFLAGS.

I mean that if your doc-diff runs:

  make USE_ASCIIDOCTOR=Yes ASCIIDOC_FLAGS=-adocdate=01/01/1970

then that will override anything the user put into config.mak. If they had some option like:

  ASCIIDOC_FLAGS = --load-path=/some/special/directory

they need for asciidoctor to run correctly on their system, then things would break for them. But we don't even have a user-facing ASCIIDOC_FLAGS now, and nobody is asking for it, so it's pretty hypothetical (I'd guess somebody in this situation would just set ASCIIDOC="asciidoctor --load-path=...", and that already doesn't work with doc-diff).

-Peff

Jeff King wrote:

Yeah, but I am implying that one is more right than the other.

It's used in the main Makefile, although in a different way.

I see how it is not consistent with the rest of the Makefiles, but I wonder why it's not being used. It's rather useful.

I know, but status quo is not an argument.

If we always did things the way we've always done things there would never be progress.

I'm aruging there's no value in giving the user the opportunity to break the build by doing `make BASIC_CFLAGS=`. Yes, it's more historically consistent, that doesn't mean it's good.

Exactly, so it's unclear how much value we get by talking about these.

Either way, I don't feel very strongly about `override ASCIIDOC_EXTRA`. I think it's superior but ASCIIDOC_FLAGS requires less changes, so I'm fine with that.

Cheers.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/doc-diff | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

On Fri, May 14, 2021 at 07:14:26AM -0500, Felipe Contreras wrote:

This line does nothing; the shell won't split whitespace here either way.

This part is necessary (and sufficient).

It would be nice to mention in the commit message why the use of $makemanflags in render_tree must remain unquoted (as I did in mine when I made the same change).

-Peff

Jeff King wrote:

Right.

Ahh, I didn't see exactly how you implemented it. Now I see it's very similar.

In order to minimize the differences in the footer.

Asciidoc automatically generates a date with format '%Y-%m-%d', while asciidoctor '%F'.

I personally prefer the latter, so only modify it for diff purposes.

Fixes tons of these:

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

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/doc-diff | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Fri, 14 May 2021 at 14:14, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Is that really the format Asciidoc uses? Not that it matters much for the purposes of this patch.

I agree completely.

Nice.

If you introduce a separate Makefile flag and incorporate Peff's patch to doc-diff "asciidoctor" and "asciidoctor-direct", you'd need to duplicate this a bit, or maybe just emit the ASCIIDOC_EXTRA outside of the whole if chain.

You could follow up with the patch below. If you'd rather keep it out of your series to avoid it ballooning, fine. I can repost it later, once the dust has settled. Don't let it hold up your work.

-- >8 --
Subject: [PATCH] doc-diff: drop --cut-footer switch

Now that our doc-diff convinces Asciidoctor to insert the exact same formatted dummy date as AsciiDoc, we can drop the --cut-footer switch. It has been useful to ignore this difference between the two tools, but it's effectively a no-op now. Similar to when we repurposed this from --cut-header-footer in 83b0b8953e ("doc-diff: replace --cut-header-footer with --cut-footer", 2019-09-16), just drop it without worrying about any kind of backwards compatibility or user-base.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 Documentation/doc-diff | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

Martin Ågren wrote:

Yes:

https://github.com/asciidoc-py/asciidoc-py/blob/main/asciidoc/asciidoc.py#L1189

Indeed.

I'll include it. Seems to fit the context.

Cheers.

There's no need to use xmlto to build the man pages when modern asciidoctor can do it by itself.

This new mode will be active only when USE_ASCIIDOCTOR is set.

Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile | 6 ++++++
 1 file changed, 6 insertions(+)

On Fri, 14 May 2021 at 14:14, Felipe Contreras <felipe.contreras@gmail.com> wrote:

May I suggest incorporating something more like brian's patch here [1], so that there's a separate knob for this thing?

The commit message is short on details and makes it sound like this is it, we're done. But then there are several patches to fix up things. Which is a good approach, so that this patch doesn't need to do several things at once. This commit message could say something about it, e.g.,

  The doc-diff here [which doc-diff? see below] is a XYZ-line diff.
  Large parts of this difference will be addressed in the following
  patches.

About the use of doc-diff: If this commit introduces a new knob rather than taking over USE_ASCIIDOCTOR=Yes, the next patch could be Peff's patch to doc-diff that compares the two asciidoctor approaches [2], and then the next few patches could diff between them. That would get the asciidoc-vs-asciidoctor differences out of the way, so you can focus on asciidoctor-vs-asciidoctor.

With a separate knob, it would feel like a lot easier decision to take something like this. There are over 11000 lines in the doc-diff after applying your series, and there's the missing boldness for literals. Maybe those differences are all great (I would be missing the bold literals, though). If this series doesn't affect someone using "vanilla" USE_ASCIIDOCTOR=Yes, we could let this thing cook in master and work incrementally on top.

[1] https://lore.kernel.org/git/20210514003104.94644-2-sandals@crustytoothpaste.net/

[2] https://lore.kernel.org/git/YJt81neO7zsGz2ah@coredump.intra.peff.net/

Martin

Martin Ågren wrote:

Sure. Do you agree with the name? (USE_ASCIIDOCTOR_MANPAGE)

Right. I'll include that.

You mean [1]. I think that belongs on the same patch. It's important that if we do have a new switch, doc-diff is able to use it.

However, I personally don't need such switch, I want to compare asciidoc-vs-asciidoctor-manpage wholesale.

I want to see *all* the diffs.

I did notice the missing boldness for literals, and I know how to fix it. It's a small hack. I also noticed a few small rendering issues.

But from my point of view after my patches this is 98% done. Most of the remaining diffs are fine, for example:

-GIT-CHECK-REF-FOR(1) Git Manual GIT-CHECK-REF-FOR(1) +GIT-CHECK-REF-FORMAT(1) Git Manual GIT-CHECK-REF-FORMAT(1)

That's clearly a bug in asciidoc+docbook. Others are things asciidoctor renders better, and most are whitespace noise.

Cheers.

[1] https://lore.kernel.org/git/YJt1%2FDO1cXNTRNxK@coredump.intra.peff.net/

Fixes the doc-diff:

-       Please see git-commit(1) for alternative ways to add content to a
-       commit.
+       Please see  for alternative ways to add content to a commit.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/asciidoctor-extensions.rb | 2 ++
 1 file changed, 2 insertions(+)

There's no real value in having two fields when one does the trick.

Also, this add the version for asciidoctor generated man pages.

Fixes the doc-diff:

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

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Currently asciidoctor doesn't convert number character references (&#xx;) correctly for man pages.

This hack fixes the issue with minimum changes elsewhere so it's easy to remove when fixed.

Fixes doc-diffs like:

            so line count cannot be shown) and there is no difference between
            indexed copy and the working tree version (if the working tree
            version were also different, binary would have been shown in place
-           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--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
            addition and one deletion).

https://github.com/asciidoctor/asciidoctor/issues/4059

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/asciidoctor-extensions.rb | 11 +++++++++++
 1 file changed, 11 insertions(+)

The docbook manpage stylesheets convert cross-references with format the 'section called “%t”'. I personally prefer the asciidoctor version, but for now add a hack to minimize the diff.

Thanks to the extensibility of Ruby we can override corresponding method in the man page converter.

This fixes doc-diffs like:

          --worktree-attributes
              Look for attributes in .gitattributes files in the working tree as
  -           well (see the section called “ATTRIBUTES”).
  +           well (see ATTRIBUTES).

This can easily be removed later once we are confortable with the asciidoctor version.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/asciidoctor-extensions.rb | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

The way asciidoctor handles links is very primitive compared to docbook. Links are simply presented in the format "#{text} <#{target}>", which may not be all that bad for the future, but pollutes the doc-diff.

By adding another modification to convert_inline_anchor() we can present links in a form very similar to docbook, diminishing the doc-diff.

This significantly reduces the doc-diff:

From:

        abysmal performance). These safety and performance issues cannot be
        backward compatibly fixed and as such, its use is not recommended.
        Please use an alternative history filtering tool such as git
-       filter-repo[1]. If you still need to use git filter-branch, please
-       carefully read the section called “SAFETY” (and the section called
-       “PERFORMANCE”) to learn about the land mines of filter-branch, and then
-       vigilantly avoid as many of the hazards listed there as reasonably
-       possible.
+       <https://github.com/newren/git-filter-repo/> filter-repo" . If you
+       still need to use git filter-branch, please carefully read the section
+       called “SAFETY” (and the section called “PERFORMANCE”) to learn about
+       the land mines of filter-branch, and then vigilantly avoid as many of
+       the hazards listed there as reasonably possible.

-NOTES
-        1. git filter-repo
-           https://github.com/newren/git-filter-repo/
-
-        2. filter-lamely
-           https://github.com/newren/git-filter-repo/blob/master/contrib/filter-repo-demos/filter-lamely
-

To:

 NOTES
-        1. git filter-repo
+       [1]    git filter-repo
               https://github.com/newren/git-filter-repo/

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/asciidoctor-extensions.rb | 13 +++++++++++++
 1 file changed, 13 insertions(+)

So that we can present relative links correctly.

Reduces the doc-diff:

   NOTES
  -        1. “Understanding history: What is a branch?”
  -           file:///$HOME/share/doc/git-doc/user-manual.html#what-is-a-branch
  +       [1]    “Understanding history: What is a branch?”
  +              user-manual.html#what-is-a-branch

   NOTES
  -        1. “Understanding history: What is a branch?”
  +       [1]    “Understanding history: What is a branch?”
                 file:///$HOME/share/doc/git-doc/user-manual.html#what-is-a-branch

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/Makefile                  | 1 +
 Documentation/asciidoctor-extensions.rb | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

There's basically nothing we need from the original orig_convert_inline_anchor(), so let's remove calls to it.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/asciidoctor-extensions.rb | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)