[RFC/NOPATCHv3 0/4] doc: asciidoctor: direct man page creation and fixes (brian's version)

[RFC/NOPATCHv3 0/4] doc: asciidoctor: direct man page creation and fixes (brian's version)

[Felipe Contreras]

I'm sending this stub series because 1. it's still in 'seen' [1], 2. my
conflicting version is not in 'seen' [2], and 3. brian has not responded
to my constructive criticism of his version [3].

This is exactly the same as what brian sent and is in
bc/doc-asciidoctor-to-man-wo-xmlto, except I have split the first patch
into three logically separate changes, which I did suggest him to do,
but he didn't.

To be crystal clear: I am *NOT* proposing these patches to be merged,
that's why I'm marking them RFC/NOPATCH so there's no confusion.

I think these should be dropped, as my series contains these changes
already, but split logically, with better commit messages, more fixes
which reduce the doc-diff, and contains patches from both Martin Ågren
and Jeff King, as well as many cleanups.

My version has 837 less lines of doc-diff (plus has the
--to-asciidoctor-direct option), and also applies cleanly to master.

On top of these patches I'm going to comment all my objections (again).

[1] https://lore.kernel.org/git/YMvhoXVBoO08ziI1@camp.crustytoothpaste.net/
[2] https://lore.kernel.org/git/20210608090026.1737348-1-felipe.contreras@gmail.com/
[3] https://lore.kernel.org/git/609ed529e2306_431272087@natae.notmuch/

brian m. carlson (4):
  doc: add an option to have Asciidoctor build man pages directly
  doc: add linkgit macros for asciidoctor
  doc: use XML-style escapes only for HTML and XML
  doc: remove GNU_ROFF option

 Documentation/Makefile                  | 25 +++++++++++++++----------
 Documentation/asciidoctor-extensions.rb |  2 ++
 Documentation/manpage-quote-apos.xsl    | 16 ----------------
 Makefile                                |  8 ++++----
 4 files changed, 21 insertions(+), 30 deletions(-)
 delete mode 100644 Documentation/manpage-quote-apos.xsl

-- 
2.32.0

[RFC/NOPATCHv3 1/4] doc: add an option to have Asciidoctor build man pages directly

[Felipe Contreras]

From: "brian m. carlson" <sandals@crustytoothpaste.net>

Asciidoctor contains a converter to generate man pages.  In some
environments, where building only the manual pages and not the other
documentation is desired, installing a toolchain for building
DocBook-based manual pages may be burdensome, and using Asciidoctor
directly may be easier, so let's add an option to build manual pages
using Asciidoctor without the DocBook toolchain.

We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't
contain proper handling of the apostrophe, which is controlled normally
by the GNU_ROFF option.  This option for the DocBook toolchain, as well
as newer versions of Asciidoctor, makes groff output an ASCII apostrophe
instead of a Unicode apostrophe in text, so as to make copy and pasting
commands easier.  These newer versions of Asciidoctor (1.5.3 and above)
detect groff and do the right thing in all cases, so the GNU_ROFF option
is obsolete in this case.

Because Asciidoctor versions before 2.0 had a few problems with man page
output, let's default this to off for now, since some common distros are
still on 1.5.  If users are using a more modern toolchain or don't care
about the rendering issues, they can enable the option.

Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
Original-patch-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/Makefile | 12 +++++++++++-
 Makefile               |  4 ++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 81d1bf7a04..d3103c3dde 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -187,6 +187,9 @@ ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
 DBLATEX_COMMON =
 XMLTO_EXTRA += --skip-validation
 XMLTO_EXTRA += -x manpage.xsl
+ifdef USE_ASCIIDOCTOR_MANPAGE
+TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage
+endif
 endif
 
 SHELL_PATH ?= $(SHELL)
@@ -325,7 +328,7 @@ mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
 		show_tool_names can_merge "* " || :' >mergetools-merge.txt && \
 	date >$@
 
-TRACK_ASCIIDOCFLAGS = $(subst ','\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK))
+TRACK_ASCIIDOCFLAGS = $(subst ','\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK):$(USE_ASCIIDOCTOR_MANPAGE))
 
 GIT-ASCIIDOCFLAGS: FORCE
 	@FLAGS='$(TRACK_ASCIIDOCFLAGS)'; \
@@ -358,9 +361,16 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf asciidoctor-extensions.rb GIT-AS
 manpage-base-url.xsl: manpage-base-url.xsl.in
 	$(QUIET_GEN)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
 
+ifdef TXT_TO_MAN
+%.1 %.5 %.7 : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
+	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
+	$(TXT_TO_MAN) -o $@+ $< && \
+	mv $@+ $@
+else
 %.1 %.5 %.7 : %.xml manpage-base-url.xsl $(wildcard manpage*.xsl)
 	$(QUIET_XMLTO)$(RM) $@ && \
 	$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
+endif
 
 %.xml : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
diff --git a/Makefile b/Makefile
index f3dc217832..48547e2c3b 100644
--- a/Makefile
+++ b/Makefile
@@ -285,6 +285,10 @@ all::
 # Define USE_ASCIIDOCTOR to use Asciidoctor instead of AsciiDoc to build the
 # documentation.
 #
+# Define USE_ASCIIDOCTOR_MANPAGE to use Asciidoctor's manual page backend
+# instead of building manual pages from DocBook (using xmlto).  Has no effect
+# unless USE_ASCIIDOCTOR is set.
+#
 # Define ASCIIDOCTOR_EXTENSIONS_LAB to point to the location of the Asciidoctor
 # Extensions Lab if you have it available.
 #
-- 
2.32.0

RE: [RFC/NOPATCHv3 1/4] doc: add an option to have Asciidoctor build man pages directly

[Felipe Contreras]

Felipe Contreras wrote:
> From: "brian m. carlson" <sandals@crustytoothpaste.net>
> 
> Asciidoctor contains a converter to generate man pages.  In some
> environments, where building only the manual pages and not the other
> documentation is desired, installing a toolchain for building
> DocBook-based manual pages may be burdensome, and using Asciidoctor
> directly may be easier, so let's add an option to build manual pages
> using Asciidoctor without the DocBook toolchain.

This suffices:

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

> We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't
> contain proper handling of the apostrophe, which is controlled normally
> by the GNU_ROFF option.

This isn't true. There's no difference from 1.5, 1.5.3, 2.0.0 or even 2.0.15.

The only fix regarding apostrophes was done by me [1], it's in the
master branch, but not in any release yet.

> This option for the DocBook toolchain, as well
> as newer versions of Asciidoctor, makes groff output an ASCII apostrophe
> instead of a Unicode apostrophe in text, so as to make copy and pasting
> commands easier.

That isn't true. GNU_ROFF is only for docbook and doesn't affect
asciidoctor in any way.

> These newer versions of Asciidoctor (1.5.3 and above)
> detect groff and do the right thing in all cases, so the GNU_ROFF option
> is obsolete in this case.

Not true. There's nothing in 1.5.3 or any version that detects groff,
or have anything to do with GNU_ROFF.

> Because Asciidoctor versions before 2.0 had a few problems with man page
> output, let's default this to off for now, since some common distros are
> still on 1.5.

There's nothing from 1.5.8 to 2.0.0 that should be of interest to the
git project.

What are these "few problems"?


The diff is correct, but 1) the original patch was done by me, and 2) I
already have the exact same version here [2].

Cheers.

[1] https://github.com/asciidoctor/asciidoctor/commit/af0ef59538fccfbdec053b82e102a240b15cdda6
[2] https://lore.kernel.org/git/20210521224452.530852-4-felipe.contreras@gmail.com/

-- 
Felipe Contreras

[RFC/NOPATCHv3 2/4] doc: add linkgit macros for asciidoctor

[Felipe Contreras]

From: "brian m. carlson" <sandals@crustytoothpaste.net>

We also need to update the code that tells Asciidoctor how to format our
linkgit macros so that it can output proper code for man pages.  Be
careful to reset the font to the previous after the change.  In order to
do so, we must reset to the previous after each font change so the
previous state at the end is the state before our inserted text, since
troff only remembers one previous font.  We insert \e before each
font-change backslash so Asciidoctor doesn't convert them into \*(rs,
the reverse solidus character, and instead leaves them as we wanted
them.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/asciidoctor-extensions.rb | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
index d906a00803..620b3d7a88 100644
--- a/Documentation/asciidoctor-extensions.rb
+++ b/Documentation/asciidoctor-extensions.rb
@@ -15,6 +15,8 @@ module Git
           "#{target}(#{attrs[1]})</ulink>"
         elsif parent.document.basebackend? 'html'
           %(<a href="#{prefix}#{target}.html">#{target}(#{attrs[1]})</a>)
+        elsif parent.document.basebackend? 'manpage'
+          %(\e\\fB#{target}\e\\fP\e\\fR(#{attrs[1]})\e\\fP)
         elsif parent.document.basebackend? 'docbook'
           "<citerefentry>\n" \
             "<refentrytitle>#{target}</refentrytitle>" \
-- 
2.32.0

RE: [RFC/NOPATCHv3 2/4] doc: add linkgit macros for asciidoctor

[Felipe Contreras]

Felipe Contreras wrote:
> From: "brian m. carlson" <sandals@crustytoothpaste.net>

> --- a/Documentation/asciidoctor-extensions.rb
> +++ b/Documentation/asciidoctor-extensions.rb
> @@ -15,6 +15,8 @@ module Git
>            "#{target}(#{attrs[1]})</ulink>"
>          elsif parent.document.basebackend? 'html'
>            %(<a href="#{prefix}#{target}.html">#{target}(#{attrs[1]})</a>)
> +        elsif parent.document.basebackend? 'manpage'
> +          %(\e\\fB#{target}\e\\fP\e\\fR(#{attrs[1]})\e\\fP)

This is the same as my version [1]:

  format = "\e\\fB%s\e\\fP(%s)"

Except my version is cleaner, I don't see the point in doing \fR \fP.
What do we gain by that?

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

-- 
Felipe Contreras

[RFC/NOPATCHv3 3/4] doc: use XML-style escapes only for HTML and XML

[Felipe Contreras]

From: "brian m. carlson" <sandals@crustytoothpaste.net>

Additionally, we don't want to use XML-style escapes for the litdd and
plus macros, so let's only use the XML-style escapes in HTML and XML and
use something different for our man pages.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index d3103c3dde..53ef100a7a 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -183,12 +183,15 @@ ASCIIDOC_HTML = xhtml5
 ASCIIDOC_DOCBOOK = docbook5
 ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
 ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
-ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
+TXT_TO_HTML += -alitdd='&\#x2d;&\#x2d;'
+TXT_TO_XML  += -alitdd='&\#x2d;&\#x2d;'
 DBLATEX_COMMON =
 XMLTO_EXTRA += --skip-validation
 XMLTO_EXTRA += -x manpage.xsl
 ifdef USE_ASCIIDOCTOR_MANPAGE
 TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage
+TXT_TO_MAN += -aplus='+'
+TXT_TO_MAN += -alitdd='\--'
 endif
 endif
 
-- 
2.32.0

RE: [RFC/NOPATCHv3 3/4] doc: use XML-style escapes only for HTML and XML

[Felipe Contreras]

Felipe Contreras wrote:
> From: "brian m. carlson" <sandals@crustytoothpaste.net>

> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -183,12 +183,15 @@ ASCIIDOC_HTML = xhtml5
>  ASCIIDOC_DOCBOOK = docbook5
>  ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
>  ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
> -ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
> +TXT_TO_HTML += -alitdd='&\#x2d;&\#x2d;'
> +TXT_TO_XML  += -alitdd='&\#x2d;&\#x2d;'
>  DBLATEX_COMMON =
>  XMLTO_EXTRA += --skip-validation
>  XMLTO_EXTRA += -x manpage.xsl
>  ifdef USE_ASCIIDOCTOR_MANPAGE
>  TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage
> +TXT_TO_MAN += -aplus='+'
> +TXT_TO_MAN += -alitdd='\--'
>  endif
>  endif
>  

My version of the workaround [1] is much cleaner, and doesn't mess with
the build system:

  if doc.backend == 'manpage'
    doc.attributes.merge!({ 'litdd' => '\--', 'plus' => '+' })
  end

Once asciidoctor issue #4059 is fixed [2] and in widespread use the
above workaround can be removed.

[1] https://lore.kernel.org/git/20210521224452.530852-7-felipe.contreras@gmail.com/
[2] https://github.com/asciidoctor/asciidoctor/issues/4059

-- 
Felipe Contreras

[RFC/NOPATCHv3 4/4] doc: remove GNU_ROFF option

[Felipe Contreras]

From: "brian m. carlson" <sandals@crustytoothpaste.net>

By default, groff converts apostrophes in troff source to Unicode
apostrophes.  This is helpful and desirable when being used as a
typesetter, since it makes the output much cleaner and more readable,
but it is a problem in manual pages, since apostrophes are often used
around shell commands and these should remain in their ASCII form for
compatibility with the shell.

Fortunately, the DocBook stylesheets contain a workaround for this case:
they detect the special .g number register, which is set only when using
groff, and they define a special macro for apostrophes based on whether
or not it is set and use that macro to write out the proper character.
As a result, the DocBook stylesheets handle all cases correctly
automatically, whether the user is using groff or not, unlike our
GNU_ROFF code.

Additionally, this functionality was implemented in 2010.  Since nobody
is shipping a mainstream Linux distribution with security support that
old anymore, we can just safely assume that the user has upgraded their
system in the past decade and remove the GNU_ROFF option and its
corresponding stylesheet altogether.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/Makefile               |  8 --------
 Documentation/manpage-quote-apos.xsl | 16 ----------------
 Makefile                             |  4 ----
 3 files changed, 28 deletions(-)
 delete mode 100644 Documentation/manpage-quote-apos.xsl

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 53ef100a7a..53a8fa9fd3 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -168,14 +168,6 @@ MAN_BASE_URL = file://$(htmldir)/
 endif
 XMLTO_EXTRA += -m manpage-base-url.xsl
 
-# If your target system uses GNU groff, it may try to render
-# apostrophes as a "pretty" apostrophe using unicode.  This breaks
-# cut&paste, so you should set GNU_ROFF to force them to be ASCII
-# apostrophes.  Unfortunately does not work with non-GNU roff.
-ifdef GNU_ROFF
-XMLTO_EXTRA += -m manpage-quote-apos.xsl
-endif
-
 ifdef USE_ASCIIDOCTOR
 ASCIIDOC = asciidoctor
 ASCIIDOC_CONF =
diff --git a/Documentation/manpage-quote-apos.xsl b/Documentation/manpage-quote-apos.xsl
deleted file mode 100644
index aeb8839f33..0000000000
--- a/Documentation/manpage-quote-apos.xsl
+++ /dev/null
@@ -1,16 +0,0 @@
-<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
-		version="1.0">
-
-<!-- work around newer groff/man setups using a prettier apostrophe
-     that unfortunately does not quote anything when cut&pasting
-     examples to the shell -->
-<xsl:template name="escape.apostrophe">
-  <xsl:param name="content"/>
-  <xsl:call-template name="string.subst">
-    <xsl:with-param name="string" select="$content"/>
-    <xsl:with-param name="target">'</xsl:with-param>
-    <xsl:with-param name="replacement">\(aq</xsl:with-param>
-  </xsl:call-template>
-</xsl:template>
-
-</xsl:stylesheet>
diff --git a/Makefile b/Makefile
index 48547e2c3b..98484ee88c 100644
--- a/Makefile
+++ b/Makefile
@@ -278,10 +278,6 @@ all::
 # Define NO_ST_BLOCKS_IN_STRUCT_STAT if your platform does not have st_blocks
 # field that counts the on-disk footprint in 512-byte blocks.
 #
-# Define GNU_ROFF if your target system uses GNU groff.  This forces
-# apostrophes to be ASCII so that cut&pasting examples to the shell
-# will work.
-#
 # Define USE_ASCIIDOCTOR to use Asciidoctor instead of AsciiDoc to build the
 # documentation.
 #
-- 
2.32.0

RE: [RFC/NOPATCHv3 4/4] doc: remove GNU_ROFF option

[Felipe Contreras]

Felipe Contreras wrote:
> From: "brian m. carlson" <sandals@crustytoothpaste.net>
> 
> By default, groff converts apostrophes in troff source to Unicode
> apostrophes.

This has nothing to do with the actual problem. As stated in groff(7)
the apostrophe ' is converted to single closing quote \(cq.

Our problem was with acute accent \(aa, not \(cq.

And it was due to docbook doing \' where it shouldn't, not groff.

> This is helpful and desirable when being used as a
> typesetter, since it makes the output much cleaner and more readable,
> but it is a problem in manual pages, since apostrophes are often used
> around shell commands and these should remain in their ASCII form for
> compatibility with the shell.

manpages should use \(aq if they want an apostrophe quote '.

> Fortunately, the DocBook stylesheets contain a workaround for this case:
> they detect the special .g number register, which is set only when using
> groff, and they define a special macro for apostrophes based on whether
> or not it is set and use that macro to write out the proper character.

Once again nothing to do with the issue.

The only way in troff source to specify an apostrophe quote is using
\(aq, but that only works in groff. Since there's no better way to
describe the same for other troff programs ' should be used for
portability.

Doing this:

  .ie \n(.g .ds Aq \(aq
  .el .ds Aq '

is not a workaround: it's a proper implementation of a generic apostrophee
quote that works correctly everywhere.

> As a result, the DocBook stylesheets handle all cases correctly
> automatically, whether the user is using groff or not, unlike our
> GNU_ROFF code.

Yes, but you are not mentioning where it was broken, and when it was
fixed.

> Additionally, this functionality was implemented in 2010.  Since nobody
> is shipping a mainstream Linux distribution with security support that
> old anymore, we can just safely assume that the user has upgraded their
> system in the past decade and remove the GNU_ROFF option and its
> corresponding stylesheet altogether.

Correct, but my version [1] goes into much more detail with less text,
it's actually acurate, and points exactly where docbook got broken,
where it was fixed, and how other projects worked around the issue.

Cheers.

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

-- 
Felipe Contreras