* [PATCH 2/2] doc: remove GNU_ROFF option
2021-05-12 2:11 ` [PATCH 1/2] doc: add an option to have Asciidoctor " brian m. carlson
@ 2021-05-12 2:11 ` brian m. carlson
2021-05-12 2:18 ` Eric Sunshine
` (2 more replies)
2021-05-12 2:48 ` [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly Bagas Sanjaya
` (3 subsequent siblings)
4 siblings, 3 replies; 45+ messages in thread
From: brian m. carlson @ 2021-05-12 2:11 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras, Martin Ågren, Bagas Sanjaya, Jeff King
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 security support for an operating system 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.
---
Documentation/Makefile | 8 --------
Documentation/manpage-quote-apos.xsl | 16 ----------------
2 files changed, 24 deletions(-)
delete mode 100644 Documentation/manpage-quote-apos.xsl
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 536d9a5f3d..f3816772cf 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -177,14 +177,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>
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH 2/2] doc: remove GNU_ROFF option
2021-05-12 2:11 ` [PATCH 2/2] doc: remove GNU_ROFF option brian m. carlson
@ 2021-05-12 2:18 ` Eric Sunshine
2021-05-12 2:28 ` brian m. carlson
2021-05-12 4:45 ` Felipe Contreras
2021-05-13 13:11 ` Martin Ågren
2 siblings, 1 reply; 45+ messages in thread
From: Eric Sunshine @ 2021-05-12 2:18 UTC (permalink / raw)
To: brian m. carlson
Cc: Git List, Felipe Contreras, Martin Ågren, Bagas Sanjaya, Jeff King
On Tue, May 11, 2021 at 10:11 PM brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> 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 security support for an operating system 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.
> ---
Missing sign-off.
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 2/2] doc: remove GNU_ROFF option
2021-05-12 2:18 ` Eric Sunshine
@ 2021-05-12 2:28 ` brian m. carlson
0 siblings, 0 replies; 45+ messages in thread
From: brian m. carlson @ 2021-05-12 2:28 UTC (permalink / raw)
To: Eric Sunshine
Cc: Git List, Felipe Contreras, Martin Ågren, Bagas Sanjaya, Jeff King
[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]
On 2021-05-12 at 02:18:54, Eric Sunshine wrote:
> On Tue, May 11, 2021 at 10:11 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > 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 security support for an operating system 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.
> > ---
>
> Missing sign-off.
Thanks. I'll add it below and if I do a reroll, I'll send out a fixed
patch.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
--
brian m. carlson (he/him or they/them)
Houston, Texas, US
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH 2/2] doc: remove GNU_ROFF option
2021-05-12 2:11 ` [PATCH 2/2] doc: remove GNU_ROFF option brian m. carlson
2021-05-12 2:18 ` Eric Sunshine
@ 2021-05-12 4:45 ` Felipe Contreras
2021-05-14 0:11 ` brian m. carlson
2021-05-13 13:11 ` Martin Ågren
2 siblings, 1 reply; 45+ messages in thread
From: Felipe Contreras @ 2021-05-12 4:45 UTC (permalink / raw)
To: brian m. carlson, git
Cc: Felipe Contreras, Martin Ågren, Bagas Sanjaya, Jeff King
brian m. carlson wrote:
> 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 security support for an operating system 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.
I'm not sure of all that, but my machine uses Arch Linux, it ships with
groff, I've never used GNU_ROFF=1, and I can copy text with apostrophes
from the genrated man pages just fine.
So this is probably fine.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 2/2] doc: remove GNU_ROFF option
2021-05-12 4:45 ` Felipe Contreras
@ 2021-05-14 0:11 ` brian m. carlson
2021-05-15 13:30 ` Felipe Contreras
0 siblings, 1 reply; 45+ messages in thread
From: brian m. carlson @ 2021-05-14 0:11 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Martin Ågren, Bagas Sanjaya, Jeff King
[-- Attachment #1: Type: text/plain, Size: 1800 bytes --]
On 2021-05-12 at 04:45:53, Felipe Contreras wrote:
> brian m. carlson wrote:
> > 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 security support for an operating system 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.
>
> I'm not sure of all that, but my machine uses Arch Linux, it ships with
> groff, I've never used GNU_ROFF=1, and I can copy text with apostrophes
> from the genrated man pages just fine.
I'll rephrase to be clearer. Solaris 10 is still security supported,
but no major Linux distro is, and I think we'll be both be fine dropping
support for OSes shipped in 2005.
I'm glad to hear confirmation that things work for you, though.
--
brian m. carlson (he/him or they/them)
Houston, Texas, US
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 2/2] doc: remove GNU_ROFF option
2021-05-14 0:11 ` brian m. carlson
@ 2021-05-15 13:30 ` Felipe Contreras
0 siblings, 0 replies; 45+ messages in thread
From: Felipe Contreras @ 2021-05-15 13:30 UTC (permalink / raw)
To: brian m. carlson, Felipe Contreras
Cc: git, Martin Ågren, Bagas Sanjaya, Jeff King
brian m. carlson wrote:
> On 2021-05-12 at 04:45:53, Felipe Contreras wrote:
> > I'm not sure of all that, but my machine uses Arch Linux, it ships with
> > groff, I've never used GNU_ROFF=1, and I can copy text with apostrophes
> > from the genrated man pages just fine.
>
> I'll rephrase to be clearer. Solaris 10 is still security supported,
> but no major Linux distro is, and I think we'll be both be fine dropping
> support for OSes shipped in 2005.
>
> I'm glad to hear confirmation that things work for you, though.
I took at deep-dive and it turns Arch Linux configures groff to convert
\' to ', so even if git was doing something wrong, I wouldn't have
noticed.
Docbook fixed their problem in 2010, and I just sent a patch for
asciidoctor to properly fix their code as well. It should work on groff
though.
The configuration is in: `/usr/share/groff/site-tmac/man.local`, if you
want to check what your system is doing.
Cheers.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 2/2] doc: remove GNU_ROFF option
2021-05-12 2:11 ` [PATCH 2/2] doc: remove GNU_ROFF option brian m. carlson
2021-05-12 2:18 ` Eric Sunshine
2021-05-12 4:45 ` Felipe Contreras
@ 2021-05-13 13:11 ` Martin Ågren
2 siblings, 0 replies; 45+ messages in thread
From: Martin Ågren @ 2021-05-13 13:11 UTC (permalink / raw)
To: brian m. carlson
Cc: Git Mailing List, Felipe Contreras, Bagas Sanjaya, Jeff King
On Wed, 12 May 2021 at 04:11, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> Additionally, this functionality was implemented in 2010. Since nobody
> is shipping security support for an operating system 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.
> ---
> Documentation/Makefile | 8 --------
> Documentation/manpage-quote-apos.xsl | 16 ----------------
> 2 files changed, 24 deletions(-)
Thanks for dropping this cruft. There's a blurb about GNU_ROFF in the
top-level Makefile as well. We should lose it here.
Martin
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 2:11 ` [PATCH 1/2] doc: add an option to have Asciidoctor " brian m. carlson
2021-05-12 2:11 ` [PATCH 2/2] doc: remove GNU_ROFF option brian m. carlson
@ 2021-05-12 2:48 ` Bagas Sanjaya
2021-05-12 5:03 ` Felipe Contreras
2021-05-13 23:24 ` brian m. carlson
2021-05-12 4:41 ` Felipe Contreras
` (2 subsequent siblings)
4 siblings, 2 replies; 45+ messages in thread
From: Bagas Sanjaya @ 2021-05-12 2:48 UTC (permalink / raw)
To: brian m. carlson, git; +Cc: Felipe Contreras, Martin Ågren, Jeff King
On 12/05/21 09.11, brian m. carlson wrote:
> From: Felipe Contreras <felipe.contreras@gmail.com>
>
> 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.
I have concern: I currently generate manpages with Asciidoctor+xmlto. Does
this change affects people using xmlto?
> 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 detect groff and
> do the right thing in all cases, so the GNU_ROFF option is obsolete in
> this case.
At what version of Asciidoctor the apostrophe handling is corrected?
> 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.
>
> 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.
Maybe when distros upgraded shipped Asciidoctor version to 2.0, we can
bump the version requirement.
> Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> I've preserved Felipe's authorship on this patch because much of it is
> his work. However, I have made some substantial changes here with which
> I suspect he will disagree, in addition to expanding on the commit
> message, so if he would prefer, I can reroll with the authorship
> changed. I have no preference myself.
>
> Documentation/Makefile | 10 ++++++++++
> Documentation/asciidoctor-extensions.rb | 2 ++
> Makefile | 3 +++
> 3 files changed, 15 insertions(+)
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 2aae4c9cbb..536d9a5f3d 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -196,6 +196,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
I think "ASCIIDOCTOR_TO_MAN" would be good alternative name here, since
this command generates manpage from asciidoctor.
> +endif
> endif
>
> SHELL_PATH ?= $(SHELL)
> @@ -367,9 +370,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/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
> index d906a00803..40fa87b121 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'
> + %(\\fB#{target}\\fP\\fR(#{attrs[1]})\\fP)
> elsif parent.document.basebackend? 'docbook'
> "<citerefentry>\n" \
> "<refentrytitle>#{target}</refentrytitle>" \
> diff --git a/Makefile b/Makefile
> index 93664d6714..cb75dec314 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -285,6 +285,9 @@ 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.
> +#
The wording should be "...instead of building manual pages from DocBook with
xmlto".
> # Define ASCIIDOCTOR_EXTENSIONS_LAB to point to the location of the Asciidoctor
> # Extensions Lab if you have it available.
> #
>
Thanks for my review.
--
An old man doll... just what I always wanted! - Clara
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 2:48 ` [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly Bagas Sanjaya
@ 2021-05-12 5:03 ` Felipe Contreras
2021-05-13 23:24 ` brian m. carlson
1 sibling, 0 replies; 45+ messages in thread
From: Felipe Contreras @ 2021-05-12 5:03 UTC (permalink / raw)
To: Bagas Sanjaya, brian m. carlson, git
Cc: Felipe Contreras, Martin Ågren, Jeff King
Bagas Sanjaya wrote:
> On 12/05/21 09.11, brian m. carlson wrote:
> > From: Felipe Contreras <felipe.contreras@gmail.com>
> >
> > 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.
>
> I have concern: I currently generate manpages with Asciidoctor+xmlto. Does
> this change affects people using xmlto?
His proposed change: no, but mine does.
> > 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 detect groff and
> > do the right thing in all cases, so the GNU_ROFF option is obsolete in
> > this case.
>
> At what version of Asciidoctor the apostrophe handling is corrected?
I will look into this, but in my opinion it's not worth complicating the
doc toolchain for ancient distributions.
The only time people are going to notice something is when:
1. They build git with USE_ASCIIDOCTOR=YesPlease
USE_ASCIIDOCTOR_MANPAGE=YesPlease
2. They use an ancient distribution
3. They use the ancient distribution's asciidoctor packages, instead of
Ruby's asciidoctor gem
4. They open a manpage generated by this process
5. They select text that specifically has an apostrophe
6. They copy this text
7. They paste this text somewhere else
Then, they *might* see some issue.
Yeah, let's not worry about about this *too much*.
> > Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> > I've preserved Felipe's authorship on this patch because much of it is
> > his work. However, I have made some substantial changes here with which
> > I suspect he will disagree, in addition to expanding on the commit
> > message, so if he would prefer, I can reroll with the authorship
> > changed. I have no preference myself.
> >
> > Documentation/Makefile | 10 ++++++++++
> > Documentation/asciidoctor-extensions.rb | 2 ++
> > Makefile | 3 +++
> > 3 files changed, 15 insertions(+)
> >
> > diff --git a/Documentation/Makefile b/Documentation/Makefile
> > index 2aae4c9cbb..536d9a5f3d 100644
> > --- a/Documentation/Makefile
> > +++ b/Documentation/Makefile
> > @@ -196,6 +196,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
> I think "ASCIIDOCTOR_TO_MAN" would be good alternative name here, since
> this command generates manpage from asciidoctor.
All the current TXT_TO_* definitions use asciidoc.
Moreover, I'm currently working on some cleanup patches to make
TXT_TO_MAN work with asciidoc + xmlto, so this is future-proof.
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -285,6 +285,9 @@ 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.
> > +#
> The wording should be "...instead of building manual pages from DocBook with
> xmlto".
That's why in my opinion it should be the other way around:
USE_ASCIIDOCTOR_XMLTO=No.
Then the expalantion is not even needed, because you can deduce it from
the name of the configuration variable.
Cheers.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 2:48 ` [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly Bagas Sanjaya
2021-05-12 5:03 ` Felipe Contreras
@ 2021-05-13 23:24 ` brian m. carlson
2021-05-14 12:58 ` Felipe Contreras
2021-05-15 13:25 ` Felipe Contreras
1 sibling, 2 replies; 45+ messages in thread
From: brian m. carlson @ 2021-05-13 23:24 UTC (permalink / raw)
To: Bagas Sanjaya; +Cc: git, Felipe Contreras, Martin Ågren, Jeff King
[-- Attachment #1: Type: text/plain, Size: 3385 bytes --]
On 2021-05-12 at 02:48:59, Bagas Sanjaya wrote:
> On 12/05/21 09.11, brian m. carlson wrote:
> > From: Felipe Contreras <felipe.contreras@gmail.com>
> >
> > 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.
>
> I have concern: I currently generate manpages with Asciidoctor+xmlto. Does
> this change affects people using xmlto?
This change adds an option to allow not using xmlto at all but instead
using just Asciidoctor to generate manual pages. If you do nothing,
you'll continue to use xmlto as before.
> > 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 detect groff and
> > do the right thing in all cases, so the GNU_ROFF option is obsolete in
> > this case.
>
> At what version of Asciidoctor the apostrophe handling is corrected?
The first released version is 1.5.3.
> > 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.
> >
> > 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.
>
> Maybe when distros upgraded shipped Asciidoctor version to 2.0, we can
> bump the version requirement.
My general policy, which need not be Git's policy (but I think is
reasonable), is that I will support the previous version of Debian and
Ubuntu LTS for a year after the new one comes out. Under that policy,
we'd wait until a year after Debian 11 (bullseye) is released.
> > diff --git a/Makefile b/Makefile
> > index 93664d6714..cb75dec314 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -285,6 +285,9 @@ 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.
> > +#
> The wording should be "...instead of building manual pages from DocBook with
> xmlto".
I can make that change. We're not using DocBook either way, with xmlto
or other tooling (e.g., a plain xsltproc), so what we have here is
accurate.
--
brian m. carlson (he/him or they/them)
Houston, Texas, US
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-13 23:24 ` brian m. carlson
@ 2021-05-14 12:58 ` Felipe Contreras
2021-05-15 13:25 ` Felipe Contreras
1 sibling, 0 replies; 45+ messages in thread
From: Felipe Contreras @ 2021-05-14 12:58 UTC (permalink / raw)
To: brian m. carlson, Bagas Sanjaya
Cc: git, Felipe Contreras, Martin Ågren, Jeff King
brian m. carlson wrote:
> On 2021-05-12 at 02:48:59, Bagas Sanjaya wrote:
> > Maybe when distros upgraded shipped Asciidoctor version to 2.0, we can
> > bump the version requirement.
>
> My general policy, which need not be Git's policy (but I think is
> reasonable), is that I will support the previous version of Debian and
> Ubuntu LTS for a year after the new one comes out. Under that policy,
> we'd wait until a year after Debian 11 (bullseye) is released.
Under that policy the supported version would be Debian 10 (buster),
which ships with Ruby 2.5. It's more than capable of running
asciidoctor.
The CI of asciidoctor tests versionof Ruby as old as 2.3, so Debian 10
is safe.
In fact, I would bet you that asciidoctor works fine in Ruby 2.1 shipped
with Debian 8 (jessie) released in 2015. Maybe users of Debian 7 would
have trouble... *maybe*... It's hard to tell because Debian doesn't
even provide package information about that release.
> > > diff --git a/Makefile b/Makefile
> > > index 93664d6714..cb75dec314 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -285,6 +285,9 @@ 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.
> > > +#
> > The wording should be "...instead of building manual pages from DocBook with
> > xmlto".
>
> I can make that change. We're not using DocBook either way, with xmlto
> or other tooling (e.g., a plain xsltproc), so what we have here is
> accurate.
Hmm...
cat Documentation/manpage.xsl
<xsl:import href="http://docbook.sourceforge.net/release/xsl-ns/current/manpages/docbook.xsl" />
That's a deliverable of the DocBook project, is it not?
--
Felipe Contreras
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-13 23:24 ` brian m. carlson
2021-05-14 12:58 ` Felipe Contreras
@ 2021-05-15 13:25 ` Felipe Contreras
1 sibling, 0 replies; 45+ messages in thread
From: Felipe Contreras @ 2021-05-15 13:25 UTC (permalink / raw)
To: brian m. carlson, Bagas Sanjaya
Cc: git, Felipe Contreras, Martin Ågren, Jeff King
brian m. carlson wrote:
> On 2021-05-12 at 02:48:59, Bagas Sanjaya wrote:
> > On 12/05/21 09.11, brian m. carlson wrote:
> > > 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 detect groff and
> > > do the right thing in all cases, so the GNU_ROFF option is obsolete in
> > > this case.
> >
> > At what version of Asciidoctor the apostrophe handling is corrected?
>
> The first released version is 1.5.3.
I just went ahead to check that, and from the very first commit [1]
asciidoctor generated quotes compatible with groff:
git filter\-branch \-\-tree\-filter \(aqrm filename\(aq HEAD
So it has *always* worked.
You can see it from the code:
gsub('\'', '\\(aq'). # apostrophe-quote
In fact, they never changed that, so it should fail in Solaris, or
anything that doesn't use groff.
I've sent them a fix [2].
What are these "newever versions" that do the right thing in all cases?
[1] https://github.com/asciidoctor/asciidoctor/commit/7bddc416c92ff9d16c721b03bda7ed80c1e4c45f
[2] https://github.com/asciidoctor/asciidoctor/pull/4060
--
Felipe Contreras
^ permalink raw reply [flat|nested] 45+ messages in thread
* RE: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 2:11 ` [PATCH 1/2] doc: add an option to have Asciidoctor " brian m. carlson
2021-05-12 2:11 ` [PATCH 2/2] doc: remove GNU_ROFF option brian m. carlson
2021-05-12 2:48 ` [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly Bagas Sanjaya
@ 2021-05-12 4:41 ` Felipe Contreras
2021-05-13 23:38 ` brian m. carlson
2021-05-12 4:43 ` Bagas Sanjaya
2021-05-12 6:22 ` Jeff King
4 siblings, 1 reply; 45+ messages in thread
From: Felipe Contreras @ 2021-05-12 4:41 UTC (permalink / raw)
To: brian m. carlson, git
Cc: Felipe Contreras, Martin Ågren, Bagas Sanjaya, Jeff King
brian m. carlson wrote:
> From: Felipe Contreras <felipe.contreras@gmail.com>
>
> 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 detect groff and
> do the right thing in all cases, so the GNU_ROFF option is obsolete in
> this case.
>
> 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.
>
> 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.
Are "some common distros" namely Debian stable *exclusively*?
If so, I would consider flipping the default the other way around,
espececially since it's only te default shipped by the Debian stable
packages (easily fixed by `gem install asciidoctor`).
> If users are using a more modern toolchain or don't care
> about the rendering issues, they can enable the option.
The other way around: if users are using an ancient distribution they
can disable the option.
> Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Commit-message-by: brian m. carlson <sandals@crustytoothpaste.net>
I certainly would not want to pretend to have written the text above.
> ---
> I've preserved Felipe's authorship on this patch because much of it is
> his work. However, I have made some substantial changes here with which
> I suspect he will disagree, in addition to expanding on the commit
> message, so if he would prefer, I can reroll with the authorship
> changed. I have no preference myself.
Hard to tell in this frankenstein commit. I'd be fine with a
Commit-message-by line.
> Documentation/Makefile | 10 ++++++++++
> Documentation/asciidoctor-extensions.rb | 2 ++
> Makefile | 3 +++
> 3 files changed, 15 insertions(+)
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 2aae4c9cbb..536d9a5f3d 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -196,6 +196,9 @@ ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
> DBLATEX_COMMON =
> XMLTO_EXTRA += --skip-validation
> XMLTO_EXTRA += -x manpage.xsl
> +ifdef USE_ASCIIDOCTOR_MANPAGE
I'd do:
ifndef USE_ASCIIDOCTOR_XMLTO
(the other way around)
> +TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage
> +endif
> endif
>
> SHELL_PATH ?= $(SHELL)
> diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
> index d906a00803..40fa87b121 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'
> + %(\\fB#{target}\\fP\\fR(#{attrs[1]})\\fP)
I still prefer my original version, especially since:
1. I suspect most git developers are familiar with printf directives:
%s.
2. Where is that \\fP coming from? I don't see that with xmlto, nor the
publicly genrated man pages[1].
3. Doesn't work on my machine without my original \e; I see
"\fBgittutorial\fR(7)".
I don't see any way this is an improvement.
Cheers.
[1] https://git.kernel.org/pub/scm/git/git-manpages.git/tree/man1/git.1
--
Felipe Contreras
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 4:41 ` Felipe Contreras
@ 2021-05-13 23:38 ` brian m. carlson
2021-05-14 19:02 ` Felipe Contreras
0 siblings, 1 reply; 45+ messages in thread
From: brian m. carlson @ 2021-05-13 23:38 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Martin Ågren, Bagas Sanjaya, Jeff King
[-- Attachment #1: Type: text/plain, Size: 4495 bytes --]
On 2021-05-12 at 04:41:41, Felipe Contreras wrote:
> brian m. carlson wrote:
> > From: Felipe Contreras <felipe.contreras@gmail.com>
> >
> > 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 detect groff and
> > do the right thing in all cases, so the GNU_ROFF option is obsolete in
> > this case.
> >
> > 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.
> >
> > 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.
>
> Are "some common distros" namely Debian stable *exclusively*?
>
> If so, I would consider flipping the default the other way around,
> espececially since it's only te default shipped by the Debian stable
> packages (easily fixed by `gem install asciidoctor`).
CentOS 7 and Ubuntu 18.04 also ship 1.5. CentOS 8 does not appear to
ship Asciidoctor at all.
> > If users are using a more modern toolchain or don't care
> > about the rendering issues, they can enable the option.
>
> The other way around: if users are using an ancient distribution they
> can disable the option.
Debian stable is not ancient. It is less than two years old, which for
a Linux distro or any operating system distribution, is not at all
considered even reasonably old.
I am not going to propose or give my approval to a change that causes
problems building Git with the distro packages on Debian stable or the
latest Ubuntu LTS, in any way, shape, or form. People should be able to
use the distro packages if that's most convenient.
> > Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
>
> Commit-message-by: brian m. carlson <sandals@crustytoothpaste.net>
>
> I certainly would not want to pretend to have written the text above.
I'll reroll with the author reset.
> > diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
> > index d906a00803..40fa87b121 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'
> > + %(\\fB#{target}\\fP\\fR(#{attrs[1]})\\fP)
>
> I still prefer my original version, especially since:
>
> 1. I suspect most git developers are familiar with printf directives:
> %s.
> 2. Where is that \\fP coming from? I don't see that with xmlto, nor the
> publicly genrated man pages[1].
That's coming from my knowledge of troff, having used it extensively
over the years, and my general hesitance to affect global state.
> 3. Doesn't work on my machine without my original \e; I see
> "\fBgittutorial\fR(7)".
Works just fine on my system using Asciidoctor 2.0.12. The \e would
insert an additional escape character and shouldn't be needed unless
we're in copy mode (which we should not be here).
--
brian m. carlson (he/him or they/them)
Houston, Texas, US
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-13 23:38 ` brian m. carlson
@ 2021-05-14 19:02 ` Felipe Contreras
0 siblings, 0 replies; 45+ messages in thread
From: Felipe Contreras @ 2021-05-14 19:02 UTC (permalink / raw)
To: brian m. carlson, Felipe Contreras
Cc: git, Martin Ågren, Bagas Sanjaya, Jeff King
brian m. carlson wrote:
> On 2021-05-12 at 04:41:41, Felipe Contreras wrote:
> > brian m. carlson wrote:
> > > since some common distros are > still on 1.5.
> >
> > Are "some common distros" namely Debian stable *exclusively*?
> >
> > If so, I would consider flipping the default the other way around,
> > espececially since it's only te default shipped by the Debian stable
> > packages (easily fixed by `gem install asciidoctor`).
>
> CentOS 7 and Ubuntu 18.04 also ship 1.5. CentOS 8 does not appear to
> ship Asciidoctor at all.
I does not matter what version of asciidoctor they *ship* with.... At all.
I happen to have accesss to an Ubuntu 18.04 machine...
time gem install --user-install asciidoctor
real 0m3.179s
It took me 3 seconds to get asciidoctor-2.0.15.
Ubuntu 18.04 ships with ruby 2.5.1p57 (2018-03-29 revision 63029). And
that's all you need.
> > > If users are using a more modern toolchain or don't care
> > > about the rendering issues, they can enable the option.
> >
> > The other way around: if users are using an ancient distribution they
> > can disable the option.
>
> Debian stable is not ancient. It is less than two years old, which for
> a Linux distro or any operating system distribution, is not at all
> considered even reasonably old.
I guess your definition of what's "old" and mine are *very* different.
But the problem doesn't come from when the distribution was released,
but when the packages *inside* the distribution were released.
> I am not going to propose or give my approval to a change that causes
> problems building Git with the distro packages on Debian stable or the
> latest Ubuntu LTS, in any way, shape, or form. People should be able to
> use the distro packages if that's most convenient.
My proposed changes do not cause any problems building Git with any
distro package on Debian stable or the latest Ubuntu LTS in any way,
shape or form.
> > > Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
> > > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> >
> > Commit-message-by: brian m. carlson <sandals@crustytoothpaste.net>
> >
> > I certainly would not want to pretend to have written the text above.
>
> I'll reroll with the author reset.
Thats is not what I just requested.
> > > diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
> > > index d906a00803..40fa87b121 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'
> > > + %(\\fB#{target}\\fP\\fR(#{attrs[1]})\\fP)
> >
> > I still prefer my original version, especially since:
> >
> > 1. I suspect most git developers are familiar with printf directives:
> > %s.
> > 2. Where is that \\fP coming from? I don't see that with xmlto, nor the
> > publicly genrated man pages[1].
>
> That's coming from my knowledge of troff, having used it extensively
> over the years, and my general hesitance to affect global state.
Good. Send it as a *separate* patch.
Most people in the mailing list are trying to **minimize** the diff
between asciidoc+docbook and asciidoctor, not piggyback improvements.
If you want to flex your troff knowledge do it as a separate patch,
without my authorship, or s-o-b line.
> > 3. Doesn't work on my machine without my original \e; I see
> > "\fBgittutorial\fR(7)".
>
> Works just fine on my system using Asciidoctor 2.0.12. The \e would
> insert an additional escape character and shouldn't be needed unless
> we're in copy mode (which we should not be here).
I fail to see how that could work since asciidoctor replaces '\'
with '\(rs'.
This is a simple test that reproduces the issue:
ruby -r asciidoctor <<\EOF | groff -Tascii -man | less
puts Asciidoctor.convert("This is a \\fBtest\\fR.", backend: :manpage)
EOF
Those that work in your machine?
This happens both with v2.0.12 and the latest master (2.0.16.dev).
Cheers.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 2:11 ` [PATCH 1/2] doc: add an option to have Asciidoctor " brian m. carlson
` (2 preceding siblings ...)
2021-05-12 4:41 ` Felipe Contreras
@ 2021-05-12 4:43 ` Bagas Sanjaya
2021-05-13 23:54 ` brian m. carlson
2021-05-12 6:22 ` Jeff King
4 siblings, 1 reply; 45+ messages in thread
From: Bagas Sanjaya @ 2021-05-12 4:43 UTC (permalink / raw)
To: brian m. carlson, git; +Cc: Felipe Contreras, Martin Ågren, Jeff King
Another review below.
On 12/05/21 09.11, brian m. carlson wrote:
> From: Felipe Contreras <felipe.contreras@gmail.com>
>> 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 detect groff and
> do the right thing in all cases, so the GNU_ROFF option is obsolete in
> this case.
>
> 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.
>
> 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>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
It is customary for multi-patches patch series to have a cover letter.
For example, when I send a patch that add corrections to an existing
patch series, I can add permalink of that series' cover letter to be
clear that my patch is applied on top of another patch series.
> diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
> index d906a00803..40fa87b121 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'
> + %(\\fB#{target}\\fP\\fR(#{attrs[1]})\\fP)
> elsif parent.document.basebackend? 'docbook'
> "<citerefentry>\n" \
> "<refentrytitle>#{target}</refentrytitle>" \
> diff --git a/Makefile b/Makefile
> index 93664d6714..cb75dec314 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -285,6 +285,9 @@ 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.
> +#
Does USE_ASCIIDOCTOR_MANPAGE imply USE_ASCIIDOCTOR?
> # Define ASCIIDOCTOR_EXTENSIONS_LAB to point to the location of the Asciidoctor
> # Extensions Lab if you have it available.
> #
>
Thanks.
--
An old man doll... just what I always wanted! - Clara
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 4:43 ` Bagas Sanjaya
@ 2021-05-13 23:54 ` brian m. carlson
0 siblings, 0 replies; 45+ messages in thread
From: brian m. carlson @ 2021-05-13 23:54 UTC (permalink / raw)
To: Bagas Sanjaya; +Cc: git, Felipe Contreras, Martin Ågren, Jeff King
[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]
On 2021-05-12 at 04:43:14, Bagas Sanjaya wrote:
> It is customary for multi-patches patch series to have a cover letter.
> For example, when I send a patch that add corrections to an existing
> patch series, I can add permalink of that series' cover letter to be
> clear that my patch is applied on top of another patch series.
I often send one, but I didn't think one was necessary in this case.
I'll send one for v2, since I need to reroll.
> > diff --git a/Makefile b/Makefile
> > index 93664d6714..cb75dec314 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -285,6 +285,9 @@ 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.
> > +#
>
> Does USE_ASCIIDOCTOR_MANPAGE imply USE_ASCIIDOCTOR?
It does not, any more than ASCIIDOCTOR_EXTENSIONS_LAB implies that. I
will update the documentation.
--
brian m. carlson (he/him or they/them)
Houston, Texas, US
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 2:11 ` [PATCH 1/2] doc: add an option to have Asciidoctor " brian m. carlson
` (3 preceding siblings ...)
2021-05-12 4:43 ` Bagas Sanjaya
@ 2021-05-12 6:22 ` Jeff King
2021-05-12 6:30 ` Jeff King
4 siblings, 1 reply; 45+ messages in thread
From: Jeff King @ 2021-05-12 6:22 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Felipe Contreras, Martin Ågren, Bagas Sanjaya
On Wed, May 12, 2021 at 02:11:37AM +0000, brian m. carlson wrote:
> @@ -367,9 +370,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
This depends on GIT-ASCIIDOCFLAGS, which is good. But I think we'd also
want to tell that file whether we are using the direct backend or not.
Otherwise, doing:
make USE_ASCIIDOCTOR=1 git.1
make USE_ASCIIDOCTOR=1 USE_ASCIIDOCTOR_MANPAGE=1 git.1
gets confused. Because git.1 is more recent than git.txt, it things
there is nothing to build in the second case. I think you want:
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 536d9a5f3d..4b66a61f51 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -337,7 +337,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)'; \
With that change, plus a patch I'll send in a minute, it's easy to run
doc-diff on the result.
> diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
> index d906a00803..40fa87b121 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'
> + %(\\fB#{target}\\fP\\fR(#{attrs[1]})\\fP)
Unfortunately, this doesn't seem to work. Diffing the rendered docs
between regular asciidoctor-then-xmlto and direct-to-manpage shows a lot
of hunks like:
For more details about the <pathspec> syntax, see the pathspec
- entry in gitglossary(7).
+ entry in \fBgitglossary\fP\fR(7)\fP.
-Peff
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 6:22 ` Jeff King
@ 2021-05-12 6:30 ` Jeff King
2021-05-12 6:59 ` Jeff King
` (2 more replies)
0 siblings, 3 replies; 45+ messages in thread
From: Jeff King @ 2021-05-12 6:30 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Felipe Contreras, Martin Ågren, Bagas Sanjaya
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).
-- >8 --
Subject: [PATCH] doc-diff: support --asciidoctor-direct mode
The new option enables both asciidoctor as well as its direct-to-manpage
mode that skips xmlto. This lets you view the rendered difference
between the two pipelines with something like:
./doc-diff --from-asciidoctor --to-asciidoctor-direct HEAD HEAD
Note that we have to add quotes when passing around $makemanflags, as it
now may contain whitespace due to multiple arguments (but the deference
inside render_tree must remain unquoted, because it wants to perform
whitespace splitting to get the individual arguments back).
Signed-off-by: Jeff King <peff@peff.net>
---
Documentation/doc-diff | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/Documentation/doc-diff b/Documentation/doc-diff
index 1694300e50..2c774ee954 100755
--- a/Documentation/doc-diff
+++ b/Documentation/doc-diff
@@ -17,10 +17,13 @@ f force rebuild; do not rely on cached results
c,clean cleanup temporary working files
from-asciidoc use asciidoc with the 'from'-commit
from-asciidoctor use asciidoctor with the 'from'-commit
+from-asciidoctor-direct use asciidoctor without xmlto for 'from'-commit
asciidoc use asciidoc with both commits
to-asciidoc use asciidoc with the 'to'-commit
to-asciidoctor use asciidoctor with the 'to'-commit
+to-asciidoctor-direct use asciidoctor without xmlto for 'to'-commit
asciidoctor use asciidoctor with both commits
+asciidoctor-direct use asciidoctor without xml for both commits
cut-footer cut away footer
"
SUBDIRECTORY_OK=1
@@ -55,6 +58,13 @@ do
--asciidoc)
from_program=-asciidoc
to_program=-asciidoc ;;
+ --from-asciidoctor-direct)
+ from_program=-asciidoctor-direct ;;
+ --to-asciidoctor-direct)
+ to_program=-asciidoctor-direct ;;
+ --asciidoctor-direct)
+ from_program=-asciidoctor-direct
+ to_program=-asciidoctor-direct ;;
--cut-footer)
cut_footer=-cut-footer ;;
--)
@@ -112,6 +122,10 @@ construct_makemanflags () {
elif test "$1" = "-asciidoctor"
then
echo USE_ASCIIDOCTOR=YesPlease
+ elif test "$1" = "-asciidoctor-direct"
+ then
+ echo USE_ASCIIDOCTOR=YesPlease
+ echo USE_ASCIIDOCTOR_MANPAGE=YesPlease
fi
}
@@ -181,6 +195,6 @@ render_tree () {
fi
}
-render_tree $from_oid $from_dir $from_makemanflags &&
-render_tree $to_oid $to_dir $to_makemanflags &&
+render_tree $from_oid $from_dir "$from_makemanflags" &&
+render_tree $to_oid $to_dir "$to_makemanflags" &&
git -C $tmp/rendered diff --no-index "$@" $from_dir $to_dir
--
2.31.1.1027.g87a751368c
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 6:30 ` Jeff King
@ 2021-05-12 6:59 ` Jeff King
2021-05-12 19:29 ` Felipe Contreras
2021-05-13 17:30 ` Martin Ågren
2021-05-12 19:53 ` Eric Sunshine
2021-05-14 15:34 ` Martin Ågren
2 siblings, 2 replies; 45+ messages in thread
From: Jeff King @ 2021-05-12 6:59 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Felipe Contreras, Martin Ågren, Bagas Sanjaya
On Wed, May 12, 2021 at 02:30:20AM -0400, Jeff King wrote:
> 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--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 "+". 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
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 6:59 ` Jeff King
@ 2021-05-12 19:29 ` Felipe Contreras
2021-05-13 17:30 ` Martin Ågren
1 sibling, 0 replies; 45+ messages in thread
From: Felipe Contreras @ 2021-05-12 19:29 UTC (permalink / raw)
To: Jeff King, brian m. carlson
Cc: git, Felipe Contreras, Martin Ågren, Bagas Sanjaya
Jeff King wrote:
> 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--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 "+". You
> + can prevent staging any addition lines by deleting them.
>
> which is a pretty bad regression.
Is it? At leat in my system both are rendered correctly.
> 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.
This is indeed a limitation of asciidoctor: manversion is ignored.
I have a fix for that. I'll send it to the asciidoctor project.
> 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).
It is not a numbered list, it is a reference. I actually prefer the (n)
version.
> 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.
That indeed looks weird. I'm not exactly sure how to fix that properly.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 6:59 ` Jeff King
2021-05-12 19:29 ` Felipe Contreras
@ 2021-05-13 17:30 ` Martin Ågren
2021-05-13 22:37 ` Felipe Contreras
1 sibling, 1 reply; 45+ messages in thread
From: Martin Ågren @ 2021-05-13 17:30 UTC (permalink / raw)
To: Jeff King
Cc: brian m. carlson, Git Mailing List, Felipe Contreras, Bagas Sanjaya
On Wed, 12 May 2021 at 08:59, Jeff King <peff@peff.net> wrote:
> 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--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 "+". You
> + can prevent staging any addition lines by deleting them.
>
> which is a pretty bad regression.
ASCIIDOC_EXTRA += -aplus='+'
ASCIIDOC_EXTRA += -alitdd='\--'
seems to have done the trick for me at one point, but Todd had some
concerns [1] that it interacted badly with the html build, so we might
need to use a less aggressive choice of makefile variable to only affect
the direct manpage generation.
[1] https://lore.kernel.org/git/20190323192756.GK4047@pobox.com/
Martin
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-13 17:30 ` Martin Ågren
@ 2021-05-13 22:37 ` Felipe Contreras
0 siblings, 0 replies; 45+ messages in thread
From: Felipe Contreras @ 2021-05-13 22:37 UTC (permalink / raw)
To: Martin Ågren, Jeff King
Cc: brian m. carlson, Git Mailing List, Felipe Contreras, Bagas Sanjaya
Martin Ågren wrote:
> On Wed, 12 May 2021 at 08:59, Jeff King <peff@peff.net> wrote:
>
> > 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--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 "+". You
> > + can prevent staging any addition lines by deleting them.
> >
> > which is a pretty bad regression.
>
> ASCIIDOC_EXTRA += -aplus='+'
> ASCIIDOC_EXTRA += -alitdd='\--'
>
> seems to have done the trick for me at one point, but Todd had some
> concerns [1] that it interacted badly with the html build, so we might
> need to use a less aggressive choice of makefile variable to only affect
> the direct manpage generation.
I don't see a point of complicating the Makefile more if we are already
passing `-r ourstuff.rb`.
One advantage of Ruby is that everything can be overriden, so we can
override the initialization of the Document object:
module Asciidoctor
class Document
alias old_initialize initialize
def initialize(data = nil, options = {})
attributes = options[:attributes]
case attributes['backend']
when 'manpage'
attributes['litdd'] = '\--'
attributes['plus'] = '+'
when 'xhtml5'
attributes['litdd'] = '--'
end
old_initialize(data, options)
end
end
end
This does the trick for me for both backends, and it simplifies the
Makefile.
However, it's a hack for what seems to be a bug in asciidoctor. I'll
report the issue.
Cheers.
--
Felipe Contreras
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 6:30 ` Jeff King
2021-05-12 6:59 ` Jeff King
@ 2021-05-12 19:53 ` Eric Sunshine
2021-05-12 22:37 ` Jeff King
2021-05-14 15:34 ` Martin Ågren
2 siblings, 1 reply; 45+ messages in thread
From: Eric Sunshine @ 2021-05-12 19:53 UTC (permalink / raw)
To: Jeff King
Cc: brian m. carlson, Git List, Felipe Contreras, Martin Ågren,
Bagas Sanjaya
On Wed, May 12, 2021 at 2:30 AM Jeff King <peff@peff.net> wrote:
> The new option enables both asciidoctor as well as its direct-to-manpage
> mode that skips xmlto. This lets you view the rendered difference
> between the two pipelines with something like:
>
> ./doc-diff --from-asciidoctor --to-asciidoctor-direct HEAD HEAD
>
> Note that we have to add quotes when passing around $makemanflags, as it
> now may contain whitespace due to multiple arguments (but the deference
I suppose you meant s/deference/dereference/ ?
> inside render_tree must remain unquoted, because it wants to perform
> whitespace splitting to get the individual arguments back).
>
> Signed-off-by: Jeff King <peff@peff.net>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 19:53 ` Eric Sunshine
@ 2021-05-12 22:37 ` Jeff King
0 siblings, 0 replies; 45+ messages in thread
From: Jeff King @ 2021-05-12 22:37 UTC (permalink / raw)
To: Eric Sunshine
Cc: brian m. carlson, Git List, Felipe Contreras, Martin Ågren,
Bagas Sanjaya
On Wed, May 12, 2021 at 03:53:09PM -0400, Eric Sunshine wrote:
> On Wed, May 12, 2021 at 2:30 AM Jeff King <peff@peff.net> wrote:
> > The new option enables both asciidoctor as well as its direct-to-manpage
> > mode that skips xmlto. This lets you view the rendered difference
> > between the two pipelines with something like:
> >
> > ./doc-diff --from-asciidoctor --to-asciidoctor-direct HEAD HEAD
> >
> > Note that we have to add quotes when passing around $makemanflags, as it
> > now may contain whitespace due to multiple arguments (but the deference
>
> I suppose you meant s/deference/dereference/ ?
Yep, thanks.
-Peff
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly
2021-05-12 6:30 ` Jeff King
2021-05-12 6:59 ` Jeff King
2021-05-12 19:53 ` Eric Sunshine
@ 2021-05-14 15:34 ` Martin Ågren
2 siblings, 0 replies; 45+ messages in thread
From: Martin Ågren @ 2021-05-14 15:34 UTC (permalink / raw)
To: Jeff King
Cc: brian m. carlson, Git Mailing List, Felipe Contreras, Bagas Sanjaya
On Wed, 12 May 2021 at 08:30, Jeff King <peff@peff.net> wrote:
>
> 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.
>
> 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).
I agree that this would be useful to have longer-term.
> -- >8 --
> Subject: [PATCH] doc-diff: support --asciidoctor-direct mode
FWIW: Reviewed-by: Martin Ågren <martin.agren@gmail.com>
^ permalink raw reply [flat|nested] 45+ messages in thread