{"thread":{"id":"55665","subject":"[PATCH] doc: use asciidoctor to build man pages directly","startedAt":"2021-05-11T22:28:56Z","lastAt":"2021-05-15T13:30:29Z","messageCount":45,"participants":["Felipe Contreras","brian m. carlson","Eric Sunshine","Bagas Sanjaya","Jeff King","Martin Ågren","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"424215","messageId":"20210511222754.417371-1-felipe.contreras@gmail.com","threadId":"55665","inReplyTo":null,"subject":"[PATCH] doc: use asciidoctor to build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-11T22:27:54Z","receivedAt":"2021-05-11T22:28:56Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"There's no need to use xmlto to build the man pages when modern\nasciidoctor can do it by itself.\n\nThis new mode will be active only when USE_ASCIIDOCTOR is set.\n\nSuggested-by: Bagas Sanjaya \nSigned-off-by: Felipe Contreras \n---\n Documentation/Makefile | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 2aae4c9cbb..0cfa88a92b 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -196,6 +196,7 @@ ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n DBLATEX_COMMON =\n XMLTO_EXTRA += --skip-validation\n XMLTO_EXTRA += -x manpage.xsl\n+TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage\n endif\n \n SHELL_PATH ?= $(SHELL)\n@@ -367,9 +368,16 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf asciidoctor-extensions.rb GIT-AS\n manpage-base-url.xsl: manpage-base-url.xsl.in\n \t$(QUIET_GEN)sed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n \n+ifdef TXT_TO_MAN\n+%.1 %.5 %.7 : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n+\t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\n+\t$(TXT_TO_MAN) -o $@+ $< && \\\n+\tmv $@+ $@\n+else\n %.1 %.5 %.7 : %.xml manpage-base-url.xsl $(wildcard manpage*.xsl)\n \t$(QUIET_XMLTO)$(RM) $@ && \\\n \t$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<\n+endif\n \n %.xml : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n \t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\n-- \n2.31.1\n\n"},{"id":"424224","messageId":"YJsSuAscwHwRo84R@camp.crustytoothpaste.net","threadId":"55665","inReplyTo":"20210511222754.417371-1-felipe.contreras@gmail.com","subject":"Re: [PATCH] doc: use asciidoctor to build man pages directly","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-11T23:26:48Z","receivedAt":"2021-05-11T23:27:04Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-05-11 at 22:27:54, Felipe Contreras wrote:\n> There's no need to use xmlto to build the man pages when modern\n> asciidoctor can do it by itself.\n> \n> This new mode will be active only when USE_ASCIIDOCTOR is set.\n> \n> Suggested-by: Bagas Sanjaya \n> Signed-off-by: Felipe Contreras \n> ---\n> Documentation/Makefile | 8 ++++++++\n> 1 file changed, 8 insertions(+)\n> \n> diff --git a/Documentation/Makefile b/Documentation/Makefile\n> index 2aae4c9cbb..0cfa88a92b 100644\n> --- a/Documentation/Makefile\n> +++ b/Documentation/Makefile\n> @@ -196,6 +196,7 @@ ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n> DBLATEX_COMMON =\n> XMLTO_EXTRA += --skip-validation\n> XMLTO_EXTRA += -x manpage.xsl\n> +TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage\n> endif\n\nAs I mentioned elsewhere, this breaks the linkgit functionality since we\ndon't have a patch for the asciidoctor-extensions.rb file and it also\ndoesn't honor GNU_ROFF, which means that copying and pasting from manual\npages is problematic on systems which use groff.\n\nI'd prefer if we put this behind an option so that just because someone\nlike me who builds with Asciidoctor doesn't have to get this behavior.\nWe may by default enable that option if the issues I mentioned above are\naddressed, but it would still be nice to have an option to enable the\nlegacy behavior, if only for those people who may be using older\nversions of Asciidoctor.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"424242","messageId":"609b2828309fc_678ff2082@natae.notmuch","threadId":"55665","inReplyTo":"YJsSuAscwHwRo84R@camp.crustytoothpaste.net","subject":"Re: [PATCH] doc: use asciidoctor to build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-12T00:58:16Z","receivedAt":"2021-05-12T00:58:20Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"brian m. carlson wrote:\n> On 2021-05-11 at 22:27:54, Felipe Contreras wrote:\n> > There's no need to use xmlto to build the man pages when modern\n> > asciidoctor can do it by itself.\n> > \n> > This new mode will be active only when USE_ASCIIDOCTOR is set.\n> > \n> > Suggested-by: Bagas Sanjaya \n> > Signed-off-by: Felipe Contreras \n> > ---\n> > Documentation/Makefile | 8 ++++++++\n> > 1 file changed, 8 insertions(+)\n> > \n> > diff --git a/Documentation/Makefile b/Documentation/Makefile\n> > index 2aae4c9cbb..0cfa88a92b 100644\n> > --- a/Documentation/Makefile\n> > +++ b/Documentation/Makefile\n> > @@ -196,6 +196,7 @@ ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n> > DBLATEX_COMMON =\n> > XMLTO_EXTRA += --skip-validation\n> > XMLTO_EXTRA += -x manpage.xsl\n> > +TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage\n> > endif\n> \n> As I mentioned elsewhere, this breaks the linkgit functionality since we\n> don't have a patch for the asciidoctor-extensions.rb file\n\nThat's an easy fix:\n\n--- a/Documentation/asciidoctor-extensions.rb\n+++ b/Documentation/asciidoctor-extensions.rb\n@@ -15,6 +15,8 @@ module Git\n \"#{target}(#{attrs[1]})\"\n elsif parent.document.basebackend? 'html'\n %(#{target}(#{attrs[1]}))\n+ elsif parent.document.basebackend? 'manpage'\n+ \"\\e\\\\fB%s\\e\\\\fR(%s)\" % [target, attrs[1]]\n elsif parent.document.basebackend? 'docbook'\n \"\\n\" \\\n \"#{target}\" \\\n\n> and it also doesn't honor GNU_ROFF, which means that copying and\n> pasting from manual pages is problematic on systems which use groff.\n\nMy system uses groff, I don't see any issues copy-pasting from manual\npages.\n\nHow exactly can I reproduce this issue?\n\n> I'd prefer if we put this behind an option so that just because someone\n> like me who builds with Asciidoctor doesn't have to get this behavior.\n> We may by default enable that option if the issues I mentioned above are\n> addressed, but it would still be nice to have an option to enable the\n> legacy behavior, if only for those people who may be using older\n> versions of Asciidoctor.\n\nSure, that would be nice, but that's not not an itch I personally have\nright now.\n\nAt the moment I'm finding too many areas of improvement with the\ndocumentation build system. Perhaps, I'll do this after I've addressed\nthose. Or somebody else could volunteer.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"424251","messageId":"20210512021138.63598-1-sandals@crustytoothpaste.net","threadId":"55665","inReplyTo":"609b2828309fc_678ff2082@natae.notmuch","subject":"[PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-12T02:11:37Z","receivedAt":"2021-05-12T02:11:51Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: Felipe Contreras \n\nAsciidoctor contains a converter to generate man pages. In some\nenvironments, where building only the manual pages and not the other\ndocumentation is desired, installing a toolchain for building\nDocBook-based manual pages may be burdensome, and using Asciidoctor\ndirectly may be easier, so let's add an option to build manual pages\nusing Asciidoctor without the DocBook toolchain.\n\nWe generally require Asciidoctor 1.5, but versions before 1.5.3 didn't\ncontain proper handling of the apostrophe, which is controlled normally\nby the GNU_ROFF option. This option for the DocBook toolchain, as well\nas newer versions of Asciidoctor, makes groff output an ASCII apostrophe\ninstead of a Unicode apostrophe in text, so as to make copy and pasting\ncommands easier. These newer versions of Asciidoctor detect groff and\ndo the right thing in all cases, so the GNU_ROFF option is obsolete in\nthis case.\n\nWe also need to update the code that tells Asciidoctor how to format our\nlinkgit macros so that it can output proper code for man pages. Be\ncareful to reset the font to the previous after the change. In order to\ndo so, we must reset to the previous after each font change so the\nprevious state at the end is the state before our inserted text, since\ntroff only remembers one previous font.\n\nBecause Asciidoctor versions before 2.0 had a few problems with man page\noutput, let's default this to off for now, since some common distros are\nstill on 1.5. If users are using a more modern toolchain or don't care\nabout the rendering issues, they can enable the option.\n\nSuggested-by: Bagas Sanjaya \nSigned-off-by: Felipe Contreras \nSigned-off-by: brian m. carlson \n---\nI've preserved Felipe's authorship on this patch because much of it is\nhis work. However, I have made some substantial changes here with which\nI suspect he will disagree, in addition to expanding on the commit\nmessage, so if he would prefer, I can reroll with the authorship\nchanged. I have no preference myself.\n\n Documentation/Makefile | 10 ++++++++++\n Documentation/asciidoctor-extensions.rb | 2 ++\n Makefile | 3 +++\n 3 files changed, 15 insertions(+)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 2aae4c9cbb..536d9a5f3d 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -196,6 +196,9 @@ ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n DBLATEX_COMMON =\n XMLTO_EXTRA += --skip-validation\n XMLTO_EXTRA += -x manpage.xsl\n+ifdef USE_ASCIIDOCTOR_MANPAGE\n+TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage\n+endif\n endif\n \n SHELL_PATH ?= $(SHELL)\n@@ -367,9 +370,16 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf asciidoctor-extensions.rb GIT-AS\n manpage-base-url.xsl: manpage-base-url.xsl.in\n \t$(QUIET_GEN)sed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n \n+ifdef TXT_TO_MAN\n+%.1 %.5 %.7 : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n+\t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\n+\t$(TXT_TO_MAN) -o $@+ $< && \\\n+\tmv $@+ $@\n+else\n %.1 %.5 %.7 : %.xml manpage-base-url.xsl $(wildcard manpage*.xsl)\n \t$(QUIET_XMLTO)$(RM) $@ && \\\n \t$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<\n+endif\n \n %.xml : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n \t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\ndiff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\nindex d906a00803..40fa87b121 100644\n--- a/Documentation/asciidoctor-extensions.rb\n+++ b/Documentation/asciidoctor-extensions.rb\n@@ -15,6 +15,8 @@ module Git\n \"#{target}(#{attrs[1]})\"\n elsif parent.document.basebackend? 'html'\n %(#{target}(#{attrs[1]}))\n+ elsif parent.document.basebackend? 'manpage'\n+ %(\\\\fB#{target}\\\\fP\\\\fR(#{attrs[1]})\\\\fP)\n elsif parent.document.basebackend? 'docbook'\n \"\\n\" \\\n \"#{target}\" \\\ndiff --git a/Makefile b/Makefile\nindex 93664d6714..cb75dec314 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -285,6 +285,9 @@ all::\n # Define USE_ASCIIDOCTOR to use Asciidoctor instead of AsciiDoc to build the\n # documentation.\n #\n+# Define USE_ASCIIDOCTOR_MANPAGE to use Asciidoctor's manual page backend\n+# instead of building manual pages from DocBook.\n+#\n # Define ASCIIDOCTOR_EXTENSIONS_LAB to point to the location of the Asciidoctor\n # Extensions Lab if you have it available.\n #\n"},{"id":"424252","messageId":"20210512021138.63598-2-sandals@crustytoothpaste.net","threadId":"55665","inReplyTo":"20210512021138.63598-1-sandals@crustytoothpaste.net","subject":"[PATCH 2/2] doc: remove GNU_ROFF option","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-12T02:11:38Z","receivedAt":"2021-05-12T02:11:51Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"By default, groff converts apostrophes in troff source to Unicode\napostrophes. This is helpful and desirable when being used as a\ntypesetter, since it makes the output much cleaner and more readable,\nbut it is a problem in manual pages, since apostrophes are often used\naround shell commands and these should remain in their ASCII form for\ncompatibility with the shell.\n\nFortunately, the DocBook stylesheets contain a workaround for this case:\nthey detect the special .g number register, which is set only when using\ngroff, and they define a special macro for apostrophes based on whether\nor not it is set and use that macro to write out the proper character.\nAs a result, the DocBook stylesheets handle all cases correctly\nautomatically, whether the user is using groff or not, unlike our\nGNU_ROFF code.\n\nAdditionally, this functionality was implemented in 2010. Since nobody\nis shipping security support for an operating system that old anymore,\nwe can just safely assume that the user has upgraded their system in the\npast decade and remove the GNU_ROFF option and its corresponding\nstylesheet altogether.\n---\n Documentation/Makefile | 8 --------\n Documentation/manpage-quote-apos.xsl | 16 ----------------\n 2 files changed, 24 deletions(-)\n delete mode 100644 Documentation/manpage-quote-apos.xsl\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 536d9a5f3d..f3816772cf 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -177,14 +177,6 @@ MAN_BASE_URL = file://$(htmldir)/\n endif\n XMLTO_EXTRA += -m manpage-base-url.xsl\n \n-# If your target system uses GNU groff, it may try to render\n-# apostrophes as a \"pretty\" apostrophe using unicode. This breaks\n-# cut&paste, so you should set GNU_ROFF to force them to be ASCII\n-# apostrophes. Unfortunately does not work with non-GNU roff.\n-ifdef GNU_ROFF\n-XMLTO_EXTRA += -m manpage-quote-apos.xsl\n-endif\n-\n ifdef USE_ASCIIDOCTOR\n ASCIIDOC = asciidoctor\n ASCIIDOC_CONF =\ndiff --git a/Documentation/manpage-quote-apos.xsl b/Documentation/manpage-quote-apos.xsl\ndeleted file mode 100644\nindex aeb8839f33..0000000000\n--- a/Documentation/manpage-quote-apos.xsl\n+++ /dev/null\n@@ -1,16 +0,0 @@\n-\n-\n-\n-\n- \n- \n- \n- '\n- \\(aq\n- \n-\n-\n-\n"},{"id":"424253","messageId":"CAPig+cQh+8Zo4mh9NtnZbGJLRRXY+zTj_9JXeMkCUz-b9nmkzw@mail.gmail.com","threadId":"55665","inReplyTo":"20210512021138.63598-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH 2/2] doc: remove GNU_ROFF option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-05-12T02:18:54Z","receivedAt":"2021-05-12T02:19:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 11, 2021 at 10:11 PM brian m. carlson\n wrote:\n> By default, groff converts apostrophes in troff source to Unicode\n> apostrophes. This is helpful and desirable when being used as a\n> typesetter, since it makes the output much cleaner and more readable,\n> but it is a problem in manual pages, since apostrophes are often used\n> around shell commands and these should remain in their ASCII form for\n> compatibility with the shell.\n>\n> Fortunately, the DocBook stylesheets contain a workaround for this case:\n> they detect the special .g number register, which is set only when using\n> groff, and they define a special macro for apostrophes based on whether\n> or not it is set and use that macro to write out the proper character.\n> As a result, the DocBook stylesheets handle all cases correctly\n> automatically, whether the user is using groff or not, unlike our\n> GNU_ROFF code.\n>\n> Additionally, this functionality was implemented in 2010. Since nobody\n> is shipping security support for an operating system that old anymore,\n> we can just safely assume that the user has upgraded their system in the\n> past decade and remove the GNU_ROFF option and its corresponding\n> stylesheet altogether.\n> ---\n\nMissing sign-off.\n"},{"id":"424255","messageId":"YJs9OvRjG3Hk0RsM@camp.crustytoothpaste.net","threadId":"55665","inReplyTo":"CAPig+cQh+8Zo4mh9NtnZbGJLRRXY+zTj_9JXeMkCUz-b9nmkzw@mail.gmail.com","subject":"Re: [PATCH 2/2] doc: remove GNU_ROFF option","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-12T02:28:10Z","receivedAt":"2021-05-12T02:28:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-05-12 at 02:18:54, Eric Sunshine wrote:\n> On Tue, May 11, 2021 at 10:11 PM brian m. carlson\n> wrote:\n> > By default, groff converts apostrophes in troff source to Unicode\n> > apostrophes. This is helpful and desirable when being used as a\n> > typesetter, since it makes the output much cleaner and more readable,\n> > but it is a problem in manual pages, since apostrophes are often used\n> > around shell commands and these should remain in their ASCII form for\n> > compatibility with the shell.\n> >\n> > Fortunately, the DocBook stylesheets contain a workaround for this case:\n> > they detect the special .g number register, which is set only when using\n> > groff, and they define a special macro for apostrophes based on whether\n> > or not it is set and use that macro to write out the proper character.\n> > As a result, the DocBook stylesheets handle all cases correctly\n> > automatically, whether the user is using groff or not, unlike our\n> > GNU_ROFF code.\n> >\n> > Additionally, this functionality was implemented in 2010. Since nobody\n> > is shipping security support for an operating system that old anymore,\n> > we can just safely assume that the user has upgraded their system in the\n> > past decade and remove the GNU_ROFF option and its corresponding\n> > stylesheet altogether.\n> > ---\n> \n> Missing sign-off.\n\nThanks. I'll add it below and if I do a reroll, I'll send out a fixed\npatch.\n\nSigned-off-by: brian m. carlson \n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"424259","messageId":"6d56412a-cc67-22fc-717f-9fa218264b40@gmail.com","threadId":"55665","inReplyTo":"20210512021138.63598-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-05-12T02:48:59Z","receivedAt":"2021-05-12T02:49:05Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 12/05/21 09.11, brian m. carlson wrote:\n> From: Felipe Contreras \n> \n> Asciidoctor contains a converter to generate man pages. In some\n> environments, where building only the manual pages and not the other\n> documentation is desired, installing a toolchain for building\n> DocBook-based manual pages may be burdensome, and using Asciidoctor\n> directly may be easier, so let's add an option to build manual pages\n> using Asciidoctor without the DocBook toolchain.\n\nI have concern: I currently generate manpages with Asciidoctor+xmlto. Does\nthis change affects people using xmlto?\n\n> We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't\n> contain proper handling of the apostrophe, which is controlled normally\n> by the GNU_ROFF option. This option for the DocBook toolchain, as well\n> as newer versions of Asciidoctor, makes groff output an ASCII apostrophe\n> instead of a Unicode apostrophe in text, so as to make copy and pasting\n> commands easier. These newer versions of Asciidoctor detect groff and\n> do the right thing in all cases, so the GNU_ROFF option is obsolete in\n> this case.\n\nAt what version of Asciidoctor the apostrophe handling is corrected?\n\n> We also need to update the code that tells Asciidoctor how to format our\n> linkgit macros so that it can output proper code for man pages. Be\n> careful to reset the font to the previous after the change. In order to\n> do so, we must reset to the previous after each font change so the\n> previous state at the end is the state before our inserted text, since\n> troff only remembers one previous font.\n> \n> Because Asciidoctor versions before 2.0 had a few problems with man page\n> output, let's default this to off for now, since some common distros are\n> still on 1.5. If users are using a more modern toolchain or don't care\n> about the rendering issues, they can enable the option.\n\nMaybe when distros upgraded shipped Asciidoctor version to 2.0, we can\nbump the version requirement.\n\n> Suggested-by: Bagas Sanjaya \n> Signed-off-by: Felipe Contreras \n> Signed-off-by: brian m. carlson \n> ---\n> I've preserved Felipe's authorship on this patch because much of it is\n> his work. However, I have made some substantial changes here with which\n> I suspect he will disagree, in addition to expanding on the commit\n> message, so if he would prefer, I can reroll with the authorship\n> changed. I have no preference myself.\n> \n> Documentation/Makefile | 10 ++++++++++\n> Documentation/asciidoctor-extensions.rb | 2 ++\n> Makefile | 3 +++\n> 3 files changed, 15 insertions(+)\n> \n> diff --git a/Documentation/Makefile b/Documentation/Makefile\n> index 2aae4c9cbb..536d9a5f3d 100644\n> --- a/Documentation/Makefile\n> +++ b/Documentation/Makefile\n> @@ -196,6 +196,9 @@ ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n> DBLATEX_COMMON =\n> XMLTO_EXTRA += --skip-validation\n> XMLTO_EXTRA += -x manpage.xsl\n> +ifdef USE_ASCIIDOCTOR_MANPAGE\n> +TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage\nI think \"ASCIIDOCTOR_TO_MAN\" would be good alternative name here, since\nthis command generates manpage from asciidoctor.\n> +endif\n> endif\n> \n> SHELL_PATH ?= $(SHELL)\n> @@ -367,9 +370,16 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf asciidoctor-extensions.rb GIT-AS\n> manpage-base-url.xsl: manpage-base-url.xsl.in\n> \t$(QUIET_GEN)sed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n> \n> +ifdef TXT_TO_MAN\n> +%.1 %.5 %.7 : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n> +\t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\n> +\t$(TXT_TO_MAN) -o $@+ $< && \\\n> +\tmv $@+ $@\n> +else\n> %.1 %.5 %.7 : %.xml manpage-base-url.xsl $(wildcard manpage*.xsl)\n> \t$(QUIET_XMLTO)$(RM) $@ && \\\n> \t$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<\n> +endif\n> \n> %.xml : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n> \t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\n> diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\n> index d906a00803..40fa87b121 100644\n> --- a/Documentation/asciidoctor-extensions.rb\n> +++ b/Documentation/asciidoctor-extensions.rb\n> @@ -15,6 +15,8 @@ module Git\n> \"#{target}(#{attrs[1]})\"\n> elsif parent.document.basebackend? 'html'\n> %(#{target}(#{attrs[1]}))\n> + elsif parent.document.basebackend? 'manpage'\n> + %(\\\\fB#{target}\\\\fP\\\\fR(#{attrs[1]})\\\\fP)\n> elsif parent.document.basebackend? 'docbook'\n> \"\\n\" \\\n> \"#{target}\" \\\n> diff --git a/Makefile b/Makefile\n> index 93664d6714..cb75dec314 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -285,6 +285,9 @@ all::\n> # Define USE_ASCIIDOCTOR to use Asciidoctor instead of AsciiDoc to build the\n> # documentation.\n> #\n> +# Define USE_ASCIIDOCTOR_MANPAGE to use Asciidoctor's manual page backend\n> +# instead of building manual pages from DocBook.\n> +#\nThe wording should be \"...instead of building manual pages from DocBook with\nxmlto\".\n> # Define ASCIIDOCTOR_EXTENSIONS_LAB to point to the location of the Asciidoctor\n> # Extensions Lab if you have it available.\n> #\n> \n\nThanks for my review.\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"424277","messageId":"609b5c85b7c61_678ff20848@natae.notmuch","threadId":"55665","inReplyTo":"20210512021138.63598-1-sandals@crustytoothpaste.net","subject":"RE: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-12T04:41:41Z","receivedAt":"2021-05-12T04:41:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"brian m. carlson wrote:\n> From: Felipe Contreras \n> \n> Asciidoctor contains a converter to generate man pages. In some\n> environments, where building only the manual pages and not the other\n> documentation is desired, installing a toolchain for building\n> DocBook-based manual pages may be burdensome, and using Asciidoctor\n> directly may be easier, so let's add an option to build manual pages\n> using Asciidoctor without the DocBook toolchain.\n> \n> We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't\n> contain proper handling of the apostrophe, which is controlled normally\n> by the GNU_ROFF option. This option for the DocBook toolchain, as well\n> as newer versions of Asciidoctor, makes groff output an ASCII apostrophe\n> instead of a Unicode apostrophe in text, so as to make copy and pasting\n> commands easier. These newer versions of Asciidoctor detect groff and\n> do the right thing in all cases, so the GNU_ROFF option is obsolete in\n> this case.\n> \n> We also need to update the code that tells Asciidoctor how to format our\n> linkgit macros so that it can output proper code for man pages. Be\n> careful to reset the font to the previous after the change. In order to\n> do so, we must reset to the previous after each font change so the\n> previous state at the end is the state before our inserted text, since\n> troff only remembers one previous font.\n> \n> Because Asciidoctor versions before 2.0 had a few problems with man page\n> output, let's default this to off for now,\n\n> since some common distros are > still on 1.5.\n\nAre \"some common distros\" namely Debian stable *exclusively*?\n\nIf so, I would consider flipping the default the other way around,\nespececially since it's only te default shipped by the Debian stable\npackages (easily fixed by `gem install asciidoctor`).\n\n> If users are using a more modern toolchain or don't care\n> about the rendering issues, they can enable the option.\n\nThe other way around: if users are using an ancient distribution they\ncan disable the option.\n\n> Suggested-by: Bagas Sanjaya \n> Signed-off-by: Felipe Contreras \n> Signed-off-by: brian m. carlson \n\nCommit-message-by: brian m. carlson \n\nI certainly would not want to pretend to have written the text above.\n\n> ---\n> I've preserved Felipe's authorship on this patch because much of it is\n> his work. However, I have made some substantial changes here with which\n> I suspect he will disagree, in addition to expanding on the commit\n> message, so if he would prefer, I can reroll with the authorship\n> changed. I have no preference myself.\n\nHard to tell in this frankenstein commit. I'd be fine with a\nCommit-message-by line.\n\n> Documentation/Makefile | 10 ++++++++++\n> Documentation/asciidoctor-extensions.rb | 2 ++\n> Makefile | 3 +++\n> 3 files changed, 15 insertions(+)\n> \n> diff --git a/Documentation/Makefile b/Documentation/Makefile\n> index 2aae4c9cbb..536d9a5f3d 100644\n> --- a/Documentation/Makefile\n> +++ b/Documentation/Makefile\n> @@ -196,6 +196,9 @@ ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n> DBLATEX_COMMON =\n> XMLTO_EXTRA += --skip-validation\n> XMLTO_EXTRA += -x manpage.xsl\n> +ifdef USE_ASCIIDOCTOR_MANPAGE\n\nI'd do:\n\n ifndef USE_ASCIIDOCTOR_XMLTO\n\n(the other way around)\n\n> +TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage\n> +endif\n> endif\n> \n> SHELL_PATH ?= $(SHELL)\n\n> diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\n> index d906a00803..40fa87b121 100644\n> --- a/Documentation/asciidoctor-extensions.rb\n> +++ b/Documentation/asciidoctor-extensions.rb\n> @@ -15,6 +15,8 @@ module Git\n> \"#{target}(#{attrs[1]})\"\n> elsif parent.document.basebackend? 'html'\n> %(#{target}(#{attrs[1]}))\n> + elsif parent.document.basebackend? 'manpage'\n> + %(\\\\fB#{target}\\\\fP\\\\fR(#{attrs[1]})\\\\fP)\n\nI still prefer my original version, especially since:\n\n 1. I suspect most git developers are familiar with printf directives:\n %s.\n 2. Where is that \\\\fP coming from? I don't see that with xmlto, nor the\n publicly genrated man pages[1].\n 3. Doesn't work on my machine without my original \\e; I see\n \"\\fBgittutorial\\fR(7)\".\n\nI don't see any way this is an improvement.\n\nCheers.\n\n[1] https://git.kernel.org/pub/scm/git/git-manpages.git/tree/man1/git.1\n\n-- \nFelipe Contreras\n"},{"id":"424278","messageId":"117a0b3b-ae7a-ffe7-feee-ae659c588920@gmail.com","threadId":"55665","inReplyTo":"20210512021138.63598-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-05-12T04:43:14Z","receivedAt":"2021-05-12T04:43:19Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"Another review below.\n\nOn 12/05/21 09.11, brian m. carlson wrote:\n> From: Felipe Contreras \n>> Asciidoctor contains a converter to generate man pages. In some\n> environments, where building only the manual pages and not the other\n> documentation is desired, installing a toolchain for building\n> DocBook-based manual pages may be burdensome, and using Asciidoctor\n> directly may be easier, so let's add an option to build manual pages\n> using Asciidoctor without the DocBook toolchain.\n> > We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't\n> contain proper handling of the apostrophe, which is controlled normally\n> by the GNU_ROFF option. This option for the DocBook toolchain, as well\n> as newer versions of Asciidoctor, makes groff output an ASCII apostrophe\n> instead of a Unicode apostrophe in text, so as to make copy and pasting\n> commands easier. These newer versions of Asciidoctor detect groff and\n> do the right thing in all cases, so the GNU_ROFF option is obsolete in\n> this case.\n> \n> We also need to update the code that tells Asciidoctor how to format our\n> linkgit macros so that it can output proper code for man pages. Be\n> careful to reset the font to the previous after the change. In order to\n> do so, we must reset to the previous after each font change so the\n> previous state at the end is the state before our inserted text, since\n> troff only remembers one previous font.\n> \n> Because Asciidoctor versions before 2.0 had a few problems with man page\n> output, let's default this to off for now, since some common distros are\n> still on 1.5. If users are using a more modern toolchain or don't care\n> about the rendering issues, they can enable the option.\n> \n> Suggested-by: Bagas Sanjaya \n> Signed-off-by: Felipe Contreras \n> Signed-off-by: brian m. carlson \n> ---\n\nIt is customary for multi-patches patch series to have a cover letter.\nFor example, when I send a patch that add corrections to an existing\npatch series, I can add permalink of that series' cover letter to be\nclear that my patch is applied on top of another patch series.\n\n> diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\n> index d906a00803..40fa87b121 100644\n> --- a/Documentation/asciidoctor-extensions.rb\n> +++ b/Documentation/asciidoctor-extensions.rb\n> @@ -15,6 +15,8 @@ module Git\n> \"#{target}(#{attrs[1]})\"\n> elsif parent.document.basebackend? 'html'\n> %(#{target}(#{attrs[1]}))\n> + elsif parent.document.basebackend? 'manpage'\n> + %(\\\\fB#{target}\\\\fP\\\\fR(#{attrs[1]})\\\\fP)\n> elsif parent.document.basebackend? 'docbook'\n> \"\\n\" \\\n> \"#{target}\" \\\n> diff --git a/Makefile b/Makefile\n> index 93664d6714..cb75dec314 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -285,6 +285,9 @@ all::\n> # Define USE_ASCIIDOCTOR to use Asciidoctor instead of AsciiDoc to build the\n> # documentation.\n> #\n> +# Define USE_ASCIIDOCTOR_MANPAGE to use Asciidoctor's manual page backend\n> +# instead of building manual pages from DocBook.\n> +#\n\nDoes USE_ASCIIDOCTOR_MANPAGE imply USE_ASCIIDOCTOR?\n\n> # Define ASCIIDOCTOR_EXTENSIONS_LAB to point to the location of the Asciidoctor\n> # Extensions Lab if you have it available.\n> #\n> \n\nThanks.\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"424279","messageId":"609b5d814b95c_678ff208d6@natae.notmuch","threadId":"55665","inReplyTo":"20210512021138.63598-2-sandals@crustytoothpaste.net","subject":"RE: [PATCH 2/2] doc: remove GNU_ROFF option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-12T04:45:53Z","receivedAt":"2021-05-12T04:46:01Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"brian m. carlson wrote:\n> By default, groff converts apostrophes in troff source to Unicode\n> apostrophes. This is helpful and desirable when being used as a\n> typesetter, since it makes the output much cleaner and more readable,\n> but it is a problem in manual pages, since apostrophes are often used\n> around shell commands and these should remain in their ASCII form for\n> compatibility with the shell.\n> \n> Fortunately, the DocBook stylesheets contain a workaround for this case:\n> they detect the special .g number register, which is set only when using\n> groff, and they define a special macro for apostrophes based on whether\n> or not it is set and use that macro to write out the proper character.\n> As a result, the DocBook stylesheets handle all cases correctly\n> automatically, whether the user is using groff or not, unlike our\n> GNU_ROFF code.\n> \n> Additionally, this functionality was implemented in 2010. Since nobody\n> is shipping security support for an operating system that old anymore,\n> we can just safely assume that the user has upgraded their system in the\n> past decade and remove the GNU_ROFF option and its corresponding\n> stylesheet altogether.\n\nI'm not sure of all that, but my machine uses Arch Linux, it ships with\ngroff, I've never used GNU_ROFF=1, and I can copy text with apostrophes\nfrom the genrated man pages just fine.\n\nSo this is probably fine.\n\n-- \nFelipe Contreras\n"},{"id":"424281","messageId":"609b618e663bd_678ff208ec@natae.notmuch","threadId":"55665","inReplyTo":"6d56412a-cc67-22fc-717f-9fa218264b40@gmail.com","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-12T05:03:10Z","receivedAt":"2021-05-12T05:03:17Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Bagas Sanjaya wrote:\n> On 12/05/21 09.11, brian m. carlson wrote:\n> > From: Felipe Contreras \n> > \n> > Asciidoctor contains a converter to generate man pages. In some\n> > environments, where building only the manual pages and not the other\n> > documentation is desired, installing a toolchain for building\n> > DocBook-based manual pages may be burdensome, and using Asciidoctor\n> > directly may be easier, so let's add an option to build manual pages\n> > using Asciidoctor without the DocBook toolchain.\n> \n> I have concern: I currently generate manpages with Asciidoctor+xmlto. Does\n> this change affects people using xmlto?\n\nHis proposed change: no, but mine does.\n\n> > We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't\n> > contain proper handling of the apostrophe, which is controlled normally\n> > by the GNU_ROFF option. This option for the DocBook toolchain, as well\n> > as newer versions of Asciidoctor, makes groff output an ASCII apostrophe\n> > instead of a Unicode apostrophe in text, so as to make copy and pasting\n> > commands easier. These newer versions of Asciidoctor detect groff and\n> > do the right thing in all cases, so the GNU_ROFF option is obsolete in\n> > this case.\n> \n> At what version of Asciidoctor the apostrophe handling is corrected?\n\nI will look into this, but in my opinion it's not worth complicating the\ndoc toolchain for ancient distributions.\n\nThe only time people are going to notice something is when:\n\n 1. They build git with USE_ASCIIDOCTOR=YesPlease\n USE_ASCIIDOCTOR_MANPAGE=YesPlease\n 2. They use an ancient distribution\n 3. They use the ancient distribution's asciidoctor packages, instead of\n Ruby's asciidoctor gem\n 4. They open a manpage generated by this process\n 5. They select text that specifically has an apostrophe\n 6. They copy this text\n 7. They paste this text somewhere else\n\nThen, they *might* see some issue.\n\nYeah, let's not worry about about this *too much*.\n\n> > Suggested-by: Bagas Sanjaya \n> > Signed-off-by: Felipe Contreras \n> > Signed-off-by: brian m. carlson \n> > ---\n> > I've preserved Felipe's authorship on this patch because much of it is\n> > his work. However, I have made some substantial changes here with which\n> > I suspect he will disagree, in addition to expanding on the commit\n> > message, so if he would prefer, I can reroll with the authorship\n> > changed. I have no preference myself.\n> > \n> > Documentation/Makefile | 10 ++++++++++\n> > Documentation/asciidoctor-extensions.rb | 2 ++\n> > Makefile | 3 +++\n> > 3 files changed, 15 insertions(+)\n> > \n> > diff --git a/Documentation/Makefile b/Documentation/Makefile\n> > index 2aae4c9cbb..536d9a5f3d 100644\n> > --- a/Documentation/Makefile\n> > +++ b/Documentation/Makefile\n> > @@ -196,6 +196,9 @@ ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n> > DBLATEX_COMMON =\n> > XMLTO_EXTRA += --skip-validation\n> > XMLTO_EXTRA += -x manpage.xsl\n> > +ifdef USE_ASCIIDOCTOR_MANPAGE\n> > +TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage\n> I think \"ASCIIDOCTOR_TO_MAN\" would be good alternative name here, since\n> this command generates manpage from asciidoctor.\n\nAll the current TXT_TO_* definitions use asciidoc.\n\nMoreover, I'm currently working on some cleanup patches to make\nTXT_TO_MAN work with asciidoc + xmlto, so this is future-proof.\n\n> > --- a/Makefile\n> > +++ b/Makefile\n> > @@ -285,6 +285,9 @@ all::\n> > # Define USE_ASCIIDOCTOR to use Asciidoctor instead of AsciiDoc to build the\n> > # documentation.\n> > #\n> > +# Define USE_ASCIIDOCTOR_MANPAGE to use Asciidoctor's manual page backend\n> > +# instead of building manual pages from DocBook.\n> > +#\n> The wording should be \"...instead of building manual pages from DocBook with\n> xmlto\".\n\nThat's why in my opinion it should be the other way around:\nUSE_ASCIIDOCTOR_XMLTO=No.\n\nThen the expalantion is not even needed, because you can deduce it from\nthe name of the configuration variable.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"424285","messageId":"YJt0Dv7HP2VnLLwv@coredump.intra.peff.net","threadId":"55665","inReplyTo":"20210512021138.63598-1-sandals@crustytoothpaste.net","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-12T06:22:06Z","receivedAt":"2021-05-12T06:22:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 12, 2021 at 02:11:37AM +0000, brian m. carlson wrote:\n\n> @@ -367,9 +370,16 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf asciidoctor-extensions.rb GIT-AS\n> manpage-base-url.xsl: manpage-base-url.xsl.in\n> \t$(QUIET_GEN)sed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n> \n> +ifdef TXT_TO_MAN\n> +%.1 %.5 %.7 : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n> +\t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\n> +\t$(TXT_TO_MAN) -o $@+ $< && \\\n> +\tmv $@+ $@\n> +else\n\nThis depends on GIT-ASCIIDOCFLAGS, which is good. But I think we'd also\nwant to tell that file whether we are using the direct backend or not.\nOtherwise, doing:\n\n make USE_ASCIIDOCTOR=1 git.1\n make USE_ASCIIDOCTOR=1 USE_ASCIIDOCTOR_MANPAGE=1 git.1\n\ngets confused. Because git.1 is more recent than git.txt, it things\nthere is nothing to build in the second case. I think you want:\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 536d9a5f3d..4b66a61f51 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -337,7 +337,7 @@ mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)\n \t\tshow_tool_names can_merge \"* \" || :' >mergetools-merge.txt && \\\n \tdate >$@\n \n-TRACK_ASCIIDOCFLAGS = $(subst ','\\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK))\n+TRACK_ASCIIDOCFLAGS = $(subst ','\\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK):$(USE_ASCIIDOCTOR_MANPAGE))\n \n GIT-ASCIIDOCFLAGS: FORCE\n \t@FLAGS='$(TRACK_ASCIIDOCFLAGS)'; \\\n\nWith that change, plus a patch I'll send in a minute, it's easy to run\ndoc-diff on the result.\n\n> diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\n> index d906a00803..40fa87b121 100644\n> --- a/Documentation/asciidoctor-extensions.rb\n> +++ b/Documentation/asciidoctor-extensions.rb\n> @@ -15,6 +15,8 @@ module Git\n> \"#{target}(#{attrs[1]})\"\n> elsif parent.document.basebackend? 'html'\n> %(#{target}(#{attrs[1]}))\n> + elsif parent.document.basebackend? 'manpage'\n> + %(\\\\fB#{target}\\\\fP\\\\fR(#{attrs[1]})\\\\fP)\n\nUnfortunately, this doesn't seem to work. Diffing the rendered docs\nbetween regular asciidoctor-then-xmlto and direct-to-manpage shows a lot\nof hunks like:\n\n For more details about the syntax, see the pathspec\n - entry in gitglossary(7).\n + entry in \\fBgitglossary\\fP\\fR(7)\\fP.\n\n-Peff\n"},{"id":"424288","messageId":"YJt1/DO1cXNTRNxK@coredump.intra.peff.net","threadId":"55665","inReplyTo":"YJt0Dv7HP2VnLLwv@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-12T06:30:20Z","receivedAt":"2021-05-12T06:30:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 12, 2021 at 02:22:06AM -0400, Jeff King wrote:\n\n> With that change, plus a patch I'll send in a minute, it's easy to run\n> doc-diff on the result.\n\nAnd here that is (note that if you don't apply the flags fix I showed,\nthen doc-diff gets confused, because it expects back-to-back runs of\n\"make\" to handle the changed flags correctly).\n\nFeel free to add it to your series if it helps (I had originally thought\nit would just be a one-off to look at the output, but there are enough\nchanges, both correctness and style, that it may be useful to have this\noption around).\n\n-- >8 --\nSubject: [PATCH] doc-diff: support --asciidoctor-direct mode\n\nThe new option enables both asciidoctor as well as its direct-to-manpage\nmode that skips xmlto. This lets you view the rendered difference\nbetween the two pipelines with something like:\n\n ./doc-diff --from-asciidoctor --to-asciidoctor-direct HEAD HEAD\n\nNote that we have to add quotes when passing around $makemanflags, as it\nnow may contain whitespace due to multiple arguments (but the deference\ninside render_tree must remain unquoted, because it wants to perform\nwhitespace splitting to get the individual arguments back).\n\nSigned-off-by: Jeff King \n---\n Documentation/doc-diff | 18 ++++++++++++++++--\n 1 file changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/doc-diff b/Documentation/doc-diff\nindex 1694300e50..2c774ee954 100755\n--- a/Documentation/doc-diff\n+++ b/Documentation/doc-diff\n@@ -17,10 +17,13 @@ f\t\t\tforce rebuild; do not rely on cached results\n c,clean\t\t\tcleanup temporary working files\n from-asciidoc\t\tuse asciidoc with the 'from'-commit\n from-asciidoctor\tuse asciidoctor with the 'from'-commit\n+from-asciidoctor-direct use asciidoctor without xmlto for 'from'-commit\n asciidoc\t\tuse asciidoc with both commits\n to-asciidoc\t\tuse asciidoc with the 'to'-commit\n to-asciidoctor\t\tuse asciidoctor with the 'to'-commit\n+to-asciidoctor-direct use asciidoctor without xmlto for 'to'-commit\n asciidoctor\t\tuse asciidoctor with both commits\n+asciidoctor-direct use asciidoctor without xml for both commits\n cut-footer\t\tcut away footer\n \"\n SUBDIRECTORY_OK=1\n@@ -55,6 +58,13 @@ do\n \t--asciidoc)\n \t\tfrom_program=-asciidoc\n \t\tto_program=-asciidoc ;;\n+\t--from-asciidoctor-direct)\n+\t\tfrom_program=-asciidoctor-direct ;;\n+\t--to-asciidoctor-direct)\n+\t\tto_program=-asciidoctor-direct ;;\n+\t--asciidoctor-direct)\n+\t\tfrom_program=-asciidoctor-direct\n+\t\tto_program=-asciidoctor-direct ;;\n \t--cut-footer)\n \t\tcut_footer=-cut-footer ;;\n \t--)\n@@ -112,6 +122,10 @@ construct_makemanflags () {\n \telif test \"$1\" = \"-asciidoctor\"\n \tthen\n \t\techo USE_ASCIIDOCTOR=YesPlease\n+\telif test \"$1\" = \"-asciidoctor-direct\"\n+\tthen\n+\t\techo USE_ASCIIDOCTOR=YesPlease\n+\t\techo USE_ASCIIDOCTOR_MANPAGE=YesPlease\n \tfi\n }\n \n@@ -181,6 +195,6 @@ render_tree () {\n \tfi\n }\n \n-render_tree $from_oid $from_dir $from_makemanflags &&\n-render_tree $to_oid $to_dir $to_makemanflags &&\n+render_tree $from_oid $from_dir \"$from_makemanflags\" &&\n+render_tree $to_oid $to_dir \"$to_makemanflags\" &&\n git -C $tmp/rendered diff --no-index \"$@\" $from_dir $to_dir\n-- \n2.31.1.1027.g87a751368c\n\n"},{"id":"424292","messageId":"YJt81neO7zsGz2ah@coredump.intra.peff.net","threadId":"55665","inReplyTo":"YJt1/DO1cXNTRNxK@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-12T06:59:34Z","receivedAt":"2021-05-12T06:59:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 12, 2021 at 02:30:20AM -0400, Jeff King wrote:\n\n> On Wed, May 12, 2021 at 02:22:06AM -0400, Jeff King wrote:\n> \n> > With that change, plus a patch I'll send in a minute, it's easy to run\n> > doc-diff on the result.\n> \n> And here that is (note that if you don't apply the flags fix I showed,\n> then doc-diff gets confused, because it expects back-to-back runs of\n> \"make\" to handle the changed flags correctly).\n> \n> Feel free to add it to your series if it helps (I had originally thought\n> it would just be a one-off to look at the output, but there are enough\n> changes, both correctness and style, that it may be useful to have this\n> option around).\n\nAdding in the \"\\e\" to the extensions string fixes many problems. Just\nskimming over the remaining changes (from asciidoctor+xmlto to\nasciidoctor direct), here are some things I see:\n\nBoth of the xmlto pipelines seem to insert extra space before a literal.\nThe direct version fixes that. E.g.:\n\n - Files to add content from. Fileglobs (e.g. *.c) can be given to\n [...]\n + Files to add content from. Fileglobs (e.g. *.c) can be given to add\n\nSo that's good. But what you can't see in the doc-diff rendered version\nis that we've lost the bolding on literals (this is from the \noption in git-add.1).\n\nAnother is that the direct version is less willing to break linkgit\ntargets across lines:\n\n - explained for the configuration variable core.quotePath (see git-\n - config(1)). See also --pathspec-file-nul and global\n + explained for the configuration variable core.quotePath (see\n + git-config(1)). See also --pathspec-file-nul and global\n\nThat seems fine to me, though it unfortunately does make the diff quite\nnoisy.\n\nWe seem to have a problem with some escape codes. E.g.:\n\n - of nothing). The other file, git-add--interactive.perl, has 403\n - lines added and 35 lines deleted if you commit what is in the\n - index, but working tree file has further modifications (one\n + of nothing). The other file, git-add--interactive.perl,\n + has 403 lines added and 35 lines deleted if you commit what is in\n + the index, but working tree file has further modifications (one\n\nand:\n\n - Added content is represented by lines beginning with \"+\". You can\n - prevent staging any addition lines by deleting them.\n + Added content is represented by lines beginning with \"+\". You\n + can prevent staging any addition lines by deleting them.\n\nwhich is a pretty bad regression.\n\nThe trailer misses the version field:\n\n -Git omitted 1970-01-01 GIT-ADD(1)\n +Git 1970-01-01 GIT-ADD(1)\n\nThe \"omitted\" is part of doc-diff's attempt to reduce noise in the\ndiff. But you can see that it's missing entirely in the direct version.\n\nThere are lots of whitespace changes for lists. They mostly seem fine\neither way. It also formats numbered lists differently:\n\n - 1. Delete the remote-tracking branches \"todo\", \"html\" and\n + (1) Delete the remote-tracking branches \"todo\", \"html\" and\n \"man\". The next fetch or pull will create them again\n unless you configure them not to. See git-fetch(1).\n - 2. Delete the \"test\" branch even if the \"master\" branch (or\n + (2) Delete the \"test\" branch even if the \"master\" branch (or\n whichever branch is currently checked out) does not have\n all commits from the test branch.\n\nI prefer the original, but could live with the latter (IIRC, this is\nsomething that can be configured via asciidoctor, but I didn't dig).\n\nThis one is quite curious (from gitworkflows(7)):\n\n - Example 1. Merge upwards\n + Rule: Merge upwards\n\nThe source looks like this:\n\n .Merge upwards\n [caption=\"Rule: \"]\n\nSo it looks like it takes the caption, rather than the phrase \"example\"\nthat I guess is coming from deep within the bowls of docbook. Both\nasciidoc and asciidoctor produce the \"Example\" text when going through\nxmlto, but both produce \"Rule\" when generating HTML. So I imagine the\nlatter was the intent, and this is a fix.\n\nLinks are a bit harder to read. E.g.:\n\n SEE ALSO\n git-check-ref-format(1), git-fetch(1), git-remote(1), “Understanding\n - history: What is a branch?”[1] in the Git User’s Manual.\n + history: What is a branch?”\" in the\n + Git User’s Manual.\n\nThere may be more lurking, but it's hard to tell given how noisy the\ndiff is.\n\n-Peff\n"},{"id":"424368","messageId":"609c2cb5a88ba_71bd1208b3@natae.notmuch","threadId":"55665","inReplyTo":"YJt81neO7zsGz2ah@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-12T19:29:57Z","receivedAt":"2021-05-12T20:36:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> We seem to have a problem with some escape codes. E.g.:\n> \n> - of nothing). The other file, git-add--interactive.perl, has 403\n> - lines added and 35 lines deleted if you commit what is in the\n> - index, but working tree file has further modifications (one\n> + of nothing). The other file, git-add--interactive.perl,\n> + has 403 lines added and 35 lines deleted if you commit what is in\n> + the index, but working tree file has further modifications (one\n> \n> and:\n> \n> - Added content is represented by lines beginning with \"+\". You can\n> - prevent staging any addition lines by deleting them.\n> + Added content is represented by lines beginning with \"+\". You\n> + can prevent staging any addition lines by deleting them.\n> \n> which is a pretty bad regression.\n\nIs it? At leat in my system both are rendered correctly.\n\n> The trailer misses the version field:\n> \n> -Git omitted 1970-01-01 GIT-ADD(1)\n> +Git 1970-01-01 GIT-ADD(1)\n> \n> The \"omitted\" is part of doc-diff's attempt to reduce noise in the\n> diff. But you can see that it's missing entirely in the direct version.\n\nThis is indeed a limitation of asciidoctor: manversion is ignored.\n\nI have a fix for that. I'll send it to the asciidoctor project.\n\n> There are lots of whitespace changes for lists. They mostly seem fine\n> either way. It also formats numbered lists differently:\n> \n> - 1. Delete the remote-tracking branches \"todo\", \"html\" and\n> + (1) Delete the remote-tracking branches \"todo\", \"html\" and\n> \"man\". The next fetch or pull will create them again\n> unless you configure them not to. See git-fetch(1).\n> - 2. Delete the \"test\" branch even if the \"master\" branch (or\n> + (2) Delete the \"test\" branch even if the \"master\" branch (or\n> whichever branch is currently checked out) does not have\n> all commits from the test branch.\n> \n> I prefer the original, but could live with the latter (IIRC, this is\n> something that can be configured via asciidoctor, but I didn't dig).\n\nIt is not a numbered list, it is a reference. I actually prefer the (n)\nversion.\n\n> Links are a bit harder to read. E.g.:\n> \n> SEE ALSO\n> git-check-ref-format(1), git-fetch(1), git-remote(1), “Understanding\n> - history: What is a branch?”[1] in the Git User’s Manual.\n> + history: What is a branch?”\" in the\n> + Git User’s Manual.\n\nThat indeed looks weird. I'm not exactly sure how to fix that properly.\n\n-- \nFelipe Contreras"},{"id":"424373","messageId":"CAPig+cQ6xw_RDmStExWGjmwuQQP+q5F5wCMtrMvtDt4GaAKXUg@mail.gmail.com","threadId":"55665","inReplyTo":"YJt1/DO1cXNTRNxK@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-05-12T19:53:09Z","receivedAt":"2021-05-12T20:45:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, May 12, 2021 at 2:30 AM Jeff King wrote:\n> The new option enables both asciidoctor as well as its direct-to-manpage\n> mode that skips xmlto. This lets you view the rendered difference\n> between the two pipelines with something like:\n>\n> ./doc-diff --from-asciidoctor --to-asciidoctor-direct HEAD HEAD\n>\n> Note that we have to add quotes when passing around $makemanflags, as it\n> now may contain whitespace due to multiple arguments (but the deference\n\nI suppose you meant s/deference/dereference/ ?\n\n> inside render_tree must remain unquoted, because it wants to perform\n> whitespace splitting to get the individual arguments back).\n>\n> Signed-off-by: Jeff King \n"},{"id":"424389","messageId":"YJxYnCg5y4DT+J9H@coredump.intra.peff.net","threadId":"55665","inReplyTo":"CAPig+cQ6xw_RDmStExWGjmwuQQP+q5F5wCMtrMvtDt4GaAKXUg@mail.gmail.com","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-12T22:37:16Z","receivedAt":"2021-05-12T23:28:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 12, 2021 at 03:53:09PM -0400, Eric Sunshine wrote:\n\n> On Wed, May 12, 2021 at 2:30 AM Jeff King wrote:\n> > The new option enables both asciidoctor as well as its direct-to-manpage\n> > mode that skips xmlto. This lets you view the rendered difference\n> > between the two pipelines with something like:\n> >\n> > ./doc-diff --from-asciidoctor --to-asciidoctor-direct HEAD HEAD\n> >\n> > Note that we have to add quotes when passing around $makemanflags, as it\n> > now may contain whitespace due to multiple arguments (but the deference\n> \n> I suppose you meant s/deference/dereference/ ?\n\nYep, thanks.\n\n-Peff\n"},{"id":"424469","messageId":"CAN0heSoQDv_fA+g7SVYDhp7Cv3nx1sYXDOWJ9cC2EiZ42Wv6VQ@mail.gmail.com","threadId":"55665","inReplyTo":"20210512021138.63598-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH 2/2] doc: remove GNU_ROFF option","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2021-05-13T13:11:45Z","receivedAt":"2021-05-13T13:12:00Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Wed, 12 May 2021 at 04:11, brian m. carlson\n wrote:\n>\n> Additionally, this functionality was implemented in 2010. Since nobody\n> is shipping security support for an operating system that old anymore,\n> we can just safely assume that the user has upgraded their system in the\n> past decade and remove the GNU_ROFF option and its corresponding\n> stylesheet altogether.\n> ---\n> Documentation/Makefile | 8 --------\n> Documentation/manpage-quote-apos.xsl | 16 ----------------\n> 2 files changed, 24 deletions(-)\n\nThanks for dropping this cruft. There's a blurb about GNU_ROFF in the\ntop-level Makefile as well. We should lose it here.\n\nMartin\n"},{"id":"424485","messageId":"CAN0heSqxFdiwF=--DaBFMqmvjMDY__hvs+u0vfc=GG1jcqZwoQ@mail.gmail.com","threadId":"55665","inReplyTo":"YJt81neO7zsGz2ah@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2021-05-13T17:30:11Z","receivedAt":"2021-05-13T17:30:28Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Wed, 12 May 2021 at 08:59, Jeff King wrote:\n\n> We seem to have a problem with some escape codes. E.g.:\n>\n> - of nothing). The other file, git-add--interactive.perl, has 403\n> - lines added and 35 lines deleted if you commit what is in the\n> - index, but working tree file has further modifications (one\n> + of nothing). The other file, git-add--interactive.perl,\n> + has 403 lines added and 35 lines deleted if you commit what is in\n> + the index, but working tree file has further modifications (one\n>\n> and:\n>\n> - Added content is represented by lines beginning with \"+\". You can\n> - prevent staging any addition lines by deleting them.\n> + Added content is represented by lines beginning with \"+\". You\n> + can prevent staging any addition lines by deleting them.\n>\n> which is a pretty bad regression.\n\nASCIIDOC_EXTRA += -aplus='+'\nASCIIDOC_EXTRA += -alitdd='\\--'\n\nseems to have done the trick for me at one point, but Todd had some\nconcerns [1] that it interacted badly with the html build, so we might\nneed to use a less aggressive choice of makefile variable to only affect\nthe direct manpage generation.\n\n[1] https://lore.kernel.org/git/20190323192756.GK4047@pobox.com/\n\nMartin\n"},{"id":"424522","messageId":"609daa1b108f3_42a0208de@natae.notmuch","threadId":"55665","inReplyTo":"CAN0heSqxFdiwF=--DaBFMqmvjMDY__hvs+u0vfc=GG1jcqZwoQ@mail.gmail.com","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-13T22:37:15Z","receivedAt":"2021-05-13T22:37:21Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Martin Ågren wrote:\n> On Wed, 12 May 2021 at 08:59, Jeff King wrote:\n> \n> > We seem to have a problem with some escape codes. E.g.:\n> >\n> > - of nothing). The other file, git-add--interactive.perl, has 403\n> > - lines added and 35 lines deleted if you commit what is in the\n> > - index, but working tree file has further modifications (one\n> > + of nothing). The other file, git-add--interactive.perl,\n> > + has 403 lines added and 35 lines deleted if you commit what is in\n> > + the index, but working tree file has further modifications (one\n> >\n> > and:\n> >\n> > - Added content is represented by lines beginning with \"+\". You can\n> > - prevent staging any addition lines by deleting them.\n> > + Added content is represented by lines beginning with \"+\". You\n> > + can prevent staging any addition lines by deleting them.\n> >\n> > which is a pretty bad regression.\n> \n> ASCIIDOC_EXTRA += -aplus='+'\n> ASCIIDOC_EXTRA += -alitdd='\\--'\n> \n> seems to have done the trick for me at one point, but Todd had some\n> concerns [1] that it interacted badly with the html build, so we might\n> need to use a less aggressive choice of makefile variable to only affect\n> the direct manpage generation.\n\nI don't see a point of complicating the Makefile more if we are already\npassing `-r ourstuff.rb`.\n\nOne advantage of Ruby is that everything can be overriden, so we can\noverride the initialization of the Document object:\n\n module Asciidoctor\n class Document\n alias old_initialize initialize\n def initialize(data = nil, options = {})\n attributes = options[:attributes]\n case attributes['backend']\n when 'manpage'\n attributes['litdd'] = '\\--'\n attributes['plus'] = '+'\n when 'xhtml5'\n attributes['litdd'] = '--'\n end\n old_initialize(data, options)\n end\n end\n end\n\nThis does the trick for me for both backends, and it simplifies the\nMakefile.\n\nHowever, it's a hack for what seems to be a bug in asciidoctor. I'll\nreport the issue.\n\nCheers.\n\n-- \nFelipe Contreras"},{"id":"424526","messageId":"YJ21Qfnk0tpPgPqu@camp.crustytoothpaste.net","threadId":"55665","inReplyTo":"6d56412a-cc67-22fc-717f-9fa218264b40@gmail.com","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-13T23:24:49Z","receivedAt":"2021-05-13T23:24:56Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-05-12 at 02:48:59, Bagas Sanjaya wrote:\n> On 12/05/21 09.11, brian m. carlson wrote:\n> > From: Felipe Contreras \n> > \n> > Asciidoctor contains a converter to generate man pages. In some\n> > environments, where building only the manual pages and not the other\n> > documentation is desired, installing a toolchain for building\n> > DocBook-based manual pages may be burdensome, and using Asciidoctor\n> > directly may be easier, so let's add an option to build manual pages\n> > using Asciidoctor without the DocBook toolchain.\n> \n> I have concern: I currently generate manpages with Asciidoctor+xmlto. Does\n> this change affects people using xmlto?\n\nThis change adds an option to allow not using xmlto at all but instead\nusing just Asciidoctor to generate manual pages. If you do nothing,\nyou'll continue to use xmlto as before.\n\n> > We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't\n> > contain proper handling of the apostrophe, which is controlled normally\n> > by the GNU_ROFF option. This option for the DocBook toolchain, as well\n> > as newer versions of Asciidoctor, makes groff output an ASCII apostrophe\n> > instead of a Unicode apostrophe in text, so as to make copy and pasting\n> > commands easier. These newer versions of Asciidoctor detect groff and\n> > do the right thing in all cases, so the GNU_ROFF option is obsolete in\n> > this case.\n> \n> At what version of Asciidoctor the apostrophe handling is corrected?\n\nThe first released version is 1.5.3.\n\n> > We also need to update the code that tells Asciidoctor how to format our\n> > linkgit macros so that it can output proper code for man pages. Be\n> > careful to reset the font to the previous after the change. In order to\n> > do so, we must reset to the previous after each font change so the\n> > previous state at the end is the state before our inserted text, since\n> > troff only remembers one previous font.\n> > \n> > Because Asciidoctor versions before 2.0 had a few problems with man page\n> > output, let's default this to off for now, since some common distros are\n> > still on 1.5. If users are using a more modern toolchain or don't care\n> > about the rendering issues, they can enable the option.\n> \n> Maybe when distros upgraded shipped Asciidoctor version to 2.0, we can\n> bump the version requirement.\n\nMy general policy, which need not be Git's policy (but I think is\nreasonable), is that I will support the previous version of Debian and\nUbuntu LTS for a year after the new one comes out. Under that policy,\nwe'd wait until a year after Debian 11 (bullseye) is released.\n\n> > diff --git a/Makefile b/Makefile\n> > index 93664d6714..cb75dec314 100644\n> > --- a/Makefile\n> > +++ b/Makefile\n> > @@ -285,6 +285,9 @@ all::\n> > # Define USE_ASCIIDOCTOR to use Asciidoctor instead of AsciiDoc to build the\n> > # documentation.\n> > #\n> > +# Define USE_ASCIIDOCTOR_MANPAGE to use Asciidoctor's manual page backend\n> > +# instead of building manual pages from DocBook.\n> > +#\n> The wording should be \"...instead of building manual pages from DocBook with\n> xmlto\".\n\nI can make that change. We're not using DocBook either way, with xmlto\nor other tooling (e.g., a plain xsltproc), so what we have here is\naccurate.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"424528","messageId":"YJ24WIpnwE4LBOMt@camp.crustytoothpaste.net","threadId":"55665","inReplyTo":"609b5c85b7c61_678ff20848@natae.notmuch","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-13T23:38:00Z","receivedAt":"2021-05-13T23:38:39Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-05-12 at 04:41:41, Felipe Contreras wrote:\n> brian m. carlson wrote:\n> > From: Felipe Contreras \n> > \n> > Asciidoctor contains a converter to generate man pages. In some\n> > environments, where building only the manual pages and not the other\n> > documentation is desired, installing a toolchain for building\n> > DocBook-based manual pages may be burdensome, and using Asciidoctor\n> > directly may be easier, so let's add an option to build manual pages\n> > using Asciidoctor without the DocBook toolchain.\n> > \n> > We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't\n> > contain proper handling of the apostrophe, which is controlled normally\n> > by the GNU_ROFF option. This option for the DocBook toolchain, as well\n> > as newer versions of Asciidoctor, makes groff output an ASCII apostrophe\n> > instead of a Unicode apostrophe in text, so as to make copy and pasting\n> > commands easier. These newer versions of Asciidoctor detect groff and\n> > do the right thing in all cases, so the GNU_ROFF option is obsolete in\n> > this case.\n> > \n> > We also need to update the code that tells Asciidoctor how to format our\n> > linkgit macros so that it can output proper code for man pages. Be\n> > careful to reset the font to the previous after the change. In order to\n> > do so, we must reset to the previous after each font change so the\n> > previous state at the end is the state before our inserted text, since\n> > troff only remembers one previous font.\n> > \n> > Because Asciidoctor versions before 2.0 had a few problems with man page\n> > output, let's default this to off for now,\n> \n> > since some common distros are > still on 1.5.\n> \n> Are \"some common distros\" namely Debian stable *exclusively*?\n> \n> If so, I would consider flipping the default the other way around,\n> espececially since it's only te default shipped by the Debian stable\n> packages (easily fixed by `gem install asciidoctor`).\n\nCentOS 7 and Ubuntu 18.04 also ship 1.5. CentOS 8 does not appear to\nship Asciidoctor at all.\n\n> > If users are using a more modern toolchain or don't care\n> > about the rendering issues, they can enable the option.\n> \n> The other way around: if users are using an ancient distribution they\n> can disable the option.\n\nDebian stable is not ancient. It is less than two years old, which for\na Linux distro or any operating system distribution, is not at all\nconsidered even reasonably old.\n\nI am not going to propose or give my approval to a change that causes\nproblems building Git with the distro packages on Debian stable or the\nlatest Ubuntu LTS, in any way, shape, or form. People should be able to\nuse the distro packages if that's most convenient.\n\n> > Suggested-by: Bagas Sanjaya \n> > Signed-off-by: Felipe Contreras \n> > Signed-off-by: brian m. carlson \n> \n> Commit-message-by: brian m. carlson \n> \n> I certainly would not want to pretend to have written the text above.\n\nI'll reroll with the author reset.\n\n> > diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\n> > index d906a00803..40fa87b121 100644\n> > --- a/Documentation/asciidoctor-extensions.rb\n> > +++ b/Documentation/asciidoctor-extensions.rb\n> > @@ -15,6 +15,8 @@ module Git\n> > \"#{target}(#{attrs[1]})\"\n> > elsif parent.document.basebackend? 'html'\n> > %(#{target}(#{attrs[1]}))\n> > + elsif parent.document.basebackend? 'manpage'\n> > + %(\\\\fB#{target}\\\\fP\\\\fR(#{attrs[1]})\\\\fP)\n> \n> I still prefer my original version, especially since:\n> \n> 1. I suspect most git developers are familiar with printf directives:\n> %s.\n> 2. Where is that \\\\fP coming from? I don't see that with xmlto, nor the\n> publicly genrated man pages[1].\n\nThat's coming from my knowledge of troff, having used it extensively\nover the years, and my general hesitance to affect global state.\n\n> 3. Doesn't work on my machine without my original \\e; I see\n> \"\\fBgittutorial\\fR(7)\".\n\nWorks just fine on my system using Asciidoctor 2.0.12. The \\e would\ninsert an additional escape character and shouldn't be needed unless\nwe're in copy mode (which we should not be here).\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"424530","messageId":"YJ28O7kBwyo4ucOC@camp.crustytoothpaste.net","threadId":"55665","inReplyTo":"117a0b3b-ae7a-ffe7-feee-ae659c588920@gmail.com","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-13T23:54:35Z","receivedAt":"2021-05-13T23:54:42Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-05-12 at 04:43:14, Bagas Sanjaya wrote:\n> It is customary for multi-patches patch series to have a cover letter.\n> For example, when I send a patch that add corrections to an existing\n> patch series, I can add permalink of that series' cover letter to be\n> clear that my patch is applied on top of another patch series.\n\nI often send one, but I didn't think one was necessary in this case.\nI'll send one for v2, since I need to reroll.\n\n> > diff --git a/Makefile b/Makefile\n> > index 93664d6714..cb75dec314 100644\n> > --- a/Makefile\n> > +++ b/Makefile\n> > @@ -285,6 +285,9 @@ all::\n> > # Define USE_ASCIIDOCTOR to use Asciidoctor instead of AsciiDoc to build the\n> > # documentation.\n> > #\n> > +# Define USE_ASCIIDOCTOR_MANPAGE to use Asciidoctor's manual page backend\n> > +# instead of building manual pages from DocBook.\n> > +#\n> \n> Does USE_ASCIIDOCTOR_MANPAGE imply USE_ASCIIDOCTOR?\n\nIt does not, any more than ASCIIDOCTOR_EXTENSIONS_LAB implies that. I\nwill update the documentation.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"424531","messageId":"YJ3ASK/LezQ86ct1@camp.crustytoothpaste.net","threadId":"55665","inReplyTo":"609b5d814b95c_678ff208d6@natae.notmuch","subject":"Re: [PATCH 2/2] doc: remove GNU_ROFF option","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-14T00:11:52Z","receivedAt":"2021-05-14T00:12:00Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-05-12 at 04:45:53, Felipe Contreras wrote:\n> brian m. carlson wrote:\n> > By default, groff converts apostrophes in troff source to Unicode\n> > apostrophes. This is helpful and desirable when being used as a\n> > typesetter, since it makes the output much cleaner and more readable,\n> > but it is a problem in manual pages, since apostrophes are often used\n> > around shell commands and these should remain in their ASCII form for\n> > compatibility with the shell.\n> > \n> > Fortunately, the DocBook stylesheets contain a workaround for this case:\n> > they detect the special .g number register, which is set only when using\n> > groff, and they define a special macro for apostrophes based on whether\n> > or not it is set and use that macro to write out the proper character.\n> > As a result, the DocBook stylesheets handle all cases correctly\n> > automatically, whether the user is using groff or not, unlike our\n> > GNU_ROFF code.\n> > \n> > Additionally, this functionality was implemented in 2010. Since nobody\n> > is shipping security support for an operating system that old anymore,\n> > we can just safely assume that the user has upgraded their system in the\n> > past decade and remove the GNU_ROFF option and its corresponding\n> > stylesheet altogether.\n> \n> I'm not sure of all that, but my machine uses Arch Linux, it ships with\n> groff, I've never used GNU_ROFF=1, and I can copy text with apostrophes\n> from the genrated man pages just fine.\n\nI'll rephrase to be clearer. Solaris 10 is still security supported,\nbut no major Linux distro is, and I think we'll be both be fine dropping\nsupport for OSes shipped in 2005.\n\nI'm glad to hear confirmation that things work for you, though.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"424532","messageId":"20210514003104.94644-1-sandals@crustytoothpaste.net","threadId":"55665","inReplyTo":"609b2828309fc_678ff2082@natae.notmuch","subject":"[PATCH v2 0/2] Asciidoctor native manpage builds","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-14T00:31:02Z","receivedAt":"2021-05-14T00:31:15Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This series introduces native support for building manual pages with\nAsciidoctor, which is faster and somewhat easier than building with the\nDocBook toolchain.\n\nThe first patch in the series is based on one by Felipe Contreras but\ndiffers from his in that we default to the old behavior, in addition to\nsome additional functionality and a more verbose commit message.\n\nThe second patch drops support for GNU_ROFF, which is now supported in a\nmore robust way by both the Asciidoctor and DocBook toolchains.\n\nChanges from v1:\n* Rephrase commit messages and Makefile text for both patches to clarify\n and avoid ambiguity.\n* Avoid using XML-style escapes in man pages, which are not XML- or\n HTML-based.\n* Clarify that USE_ASCIIDOCTOR_MANPAGE requires USE_ASCIIDOCTOR to be\n functional.\n* Escape linkgit macro output to avoid backslashes being needlessly\n converted.\n* Remove leftover reference to GNU_ROFF.\n\nbrian m. carlson (2):\n doc: add an option to have Asciidoctor build man pages directly\n doc: remove GNU_ROFF option\n\n Documentation/Makefile | 25 +++++++++++++++----------\n Documentation/asciidoctor-extensions.rb | 2 ++\n Documentation/manpage-quote-apos.xsl | 16 ----------------\n Makefile | 8 ++++----\n 4 files changed, 21 insertions(+), 30 deletions(-)\n delete mode 100644 Documentation/manpage-quote-apos.xsl\n\nDiff-intervalle contre v1 :\n1: 9ba34d9ec9 ! 1: 0d5f8f1b80 doc: add an option to have Asciidoctor build man pages directly\n @@\n ## Metadata ##\n -Author: Felipe Contreras \n +Author: brian m. carlson \n \n ## Commit message ##\n doc: add an option to have Asciidoctor build man pages directly\n @@ Commit message\n by the GNU_ROFF option. This option for the DocBook toolchain, as well\n as newer versions of Asciidoctor, makes groff output an ASCII apostrophe\n instead of a Unicode apostrophe in text, so as to make copy and pasting\n - commands easier. These newer versions of Asciidoctor detect groff and\n - do the right thing in all cases, so the GNU_ROFF option is obsolete in\n - this case.\n + commands easier. These newer versions of Asciidoctor (1.5.3 and above)\n + detect groff and do the right thing in all cases, so the GNU_ROFF option\n + is obsolete in this case.\n \n We also need to update the code that tells Asciidoctor how to format our\n linkgit macros so that it can output proper code for man pages. Be\n careful to reset the font to the previous after the change. In order to\n do so, we must reset to the previous after each font change so the\n previous state at the end is the state before our inserted text, since\n - troff only remembers one previous font.\n + troff only remembers one previous font. We insert \\e before each\n + font-change backslash so Asciidoctor doesn't convert them into \\*(rs,\n + the reverse solidus character, and instead leaves them as we wanted\n + them.\n +\n + Additionally, we don't want to use XML-style escapes for the litdd and\n + plus macros, so let's only use the XML-style escapes in HTML and XML and\n + use something different for our man pages.\n \n Because Asciidoctor versions before 2.0 had a few problems with man page\n output, let's default this to off for now, since some common distros are\n @@ Commit message\n Signed-off-by: brian m. carlson \n \n ## Documentation/Makefile ##\n -@@ Documentation/Makefile: ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n +@@ Documentation/Makefile: ASCIIDOC_HTML = xhtml5\n + ASCIIDOC_DOCBOOK = docbook5\n + ASCIIDOC_EXTRA += -acompat-mode -atabsize=8\n + ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions\n +-ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n ++TXT_TO_HTML += -alitdd='&\\#x2d;&\\#x2d;'\n ++TXT_TO_XML += -alitdd='&\\#x2d;&\\#x2d;'\n DBLATEX_COMMON =\n XMLTO_EXTRA += --skip-validation\n XMLTO_EXTRA += -x manpage.xsl\n +ifdef USE_ASCIIDOCTOR_MANPAGE\n +TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage\n ++TXT_TO_MAN += -aplus='+'\n ++TXT_TO_MAN += -alitdd='\\--'\n +endif\n endif\n \n SHELL_PATH ?= $(SHELL)\n +@@ Documentation/Makefile: mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)\n + \t\tshow_tool_names can_merge \"* \" || :' >mergetools-merge.txt && \\\n + \tdate >$@\n + \n +-TRACK_ASCIIDOCFLAGS = $(subst ','\\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK))\n ++TRACK_ASCIIDOCFLAGS = $(subst ','\\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK):$(USE_ASCIIDOCTOR_MANPAGE))\n + \n + GIT-ASCIIDOCFLAGS: FORCE\n + \t@FLAGS='$(TRACK_ASCIIDOCFLAGS)'; \\\n @@ Documentation/Makefile: $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf asciidoctor-extensions.rb GIT-AS\n manpage-base-url.xsl: manpage-base-url.xsl.in\n \t$(QUIET_GEN)sed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n @@ Documentation/asciidoctor-extensions.rb: module Git\n elsif parent.document.basebackend? 'html'\n %(#{target}(#{attrs[1]}))\n + elsif parent.document.basebackend? 'manpage'\n -+ %(\\\\fB#{target}\\\\fP\\\\fR(#{attrs[1]})\\\\fP)\n ++ %(\\e\\\\fB#{target}\\e\\\\fP\\e\\\\fR(#{attrs[1]})\\e\\\\fP)\n elsif parent.document.basebackend? 'docbook'\n \"\\n\" \\\n \"#{target}\" \\\n @@ Makefile: all::\n # documentation.\n #\n +# Define USE_ASCIIDOCTOR_MANPAGE to use Asciidoctor's manual page backend\n -+# instead of building manual pages from DocBook.\n ++# instead of building manual pages from DocBook (using xmlto). Has no effect\n ++# unless USE_ASCIIDOCTOR is set.\n +#\n # Define ASCIIDOCTOR_EXTENSIONS_LAB to point to the location of the Asciidoctor\n # Extensions Lab if you have it available.\n2: c0a21dd700 ! 2: b12a068f5b doc: remove GNU_ROFF option\n @@ Commit message\n GNU_ROFF code.\n \n Additionally, this functionality was implemented in 2010. Since nobody\n - is shipping security support for an operating system that old anymore,\n - we can just safely assume that the user has upgraded their system in the\n - past decade and remove the GNU_ROFF option and its corresponding\n - stylesheet altogether.\n + is shipping a mainstream Linux distribution with security support that\n + old anymore, we can just safely assume that the user has upgraded their\n + system in the past decade and remove the GNU_ROFF option and its\n + corresponding stylesheet altogether.\n \n Signed-off-by: brian m. carlson \n \n @@ Documentation/manpage-quote-apos.xsl (deleted)\n -\n -\n -\n +\n + ## Makefile ##\n +@@ Makefile: all::\n + # Define NO_ST_BLOCKS_IN_STRUCT_STAT if your platform does not have st_blocks\n + # field that counts the on-disk footprint in 512-byte blocks.\n + #\n +-# Define GNU_ROFF if your target system uses GNU groff. This forces\n +-# apostrophes to be ASCII so that cut&pasting examples to the shell\n +-# will work.\n +-#\n + # Define USE_ASCIIDOCTOR to use Asciidoctor instead of AsciiDoc to build the\n + # documentation.\n + #\n"},{"id":"424533","messageId":"20210514003104.94644-2-sandals@crustytoothpaste.net","threadId":"55665","inReplyTo":"20210514003104.94644-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-14T00:31:03Z","receivedAt":"2021-05-14T00:31:15Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Asciidoctor contains a converter to generate man pages. In some\nenvironments, where building only the manual pages and not the other\ndocumentation is desired, installing a toolchain for building\nDocBook-based manual pages may be burdensome, and using Asciidoctor\ndirectly may be easier, so let's add an option to build manual pages\nusing Asciidoctor without the DocBook toolchain.\n\nWe generally require Asciidoctor 1.5, but versions before 1.5.3 didn't\ncontain proper handling of the apostrophe, which is controlled normally\nby the GNU_ROFF option. This option for the DocBook toolchain, as well\nas newer versions of Asciidoctor, makes groff output an ASCII apostrophe\ninstead of a Unicode apostrophe in text, so as to make copy and pasting\ncommands easier. These newer versions of Asciidoctor (1.5.3 and above)\ndetect groff and do the right thing in all cases, so the GNU_ROFF option\nis obsolete in this case.\n\nWe also need to update the code that tells Asciidoctor how to format our\nlinkgit macros so that it can output proper code for man pages. Be\ncareful to reset the font to the previous after the change. In order to\ndo so, we must reset to the previous after each font change so the\nprevious state at the end is the state before our inserted text, since\ntroff only remembers one previous font. We insert \\e before each\nfont-change backslash so Asciidoctor doesn't convert them into \\*(rs,\nthe reverse solidus character, and instead leaves them as we wanted\nthem.\n\nAdditionally, we don't want to use XML-style escapes for the litdd and\nplus macros, so let's only use the XML-style escapes in HTML and XML and\nuse something different for our man pages.\n\nBecause Asciidoctor versions before 2.0 had a few problems with man page\noutput, let's default this to off for now, since some common distros are\nstill on 1.5. If users are using a more modern toolchain or don't care\nabout the rendering issues, they can enable the option.\n\nSuggested-by: Bagas Sanjaya \nSigned-off-by: Felipe Contreras \nSigned-off-by: brian m. carlson \n---\n Documentation/Makefile | 17 +++++++++++++++--\n Documentation/asciidoctor-extensions.rb | 2 ++\n Makefile | 4 ++++\n 3 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 2aae4c9cbb..891181c0f3 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -192,10 +192,16 @@ ASCIIDOC_HTML = xhtml5\n ASCIIDOC_DOCBOOK = docbook5\n ASCIIDOC_EXTRA += -acompat-mode -atabsize=8\n ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions\n-ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n+TXT_TO_HTML += -alitdd='&\\#x2d;&\\#x2d;'\n+TXT_TO_XML += -alitdd='&\\#x2d;&\\#x2d;'\n DBLATEX_COMMON =\n XMLTO_EXTRA += --skip-validation\n XMLTO_EXTRA += -x manpage.xsl\n+ifdef USE_ASCIIDOCTOR_MANPAGE\n+TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage\n+TXT_TO_MAN += -aplus='+'\n+TXT_TO_MAN += -alitdd='\\--'\n+endif\n endif\n \n SHELL_PATH ?= $(SHELL)\n@@ -334,7 +340,7 @@ mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)\n \t\tshow_tool_names can_merge \"* \" || :' >mergetools-merge.txt && \\\n \tdate >$@\n \n-TRACK_ASCIIDOCFLAGS = $(subst ','\\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK))\n+TRACK_ASCIIDOCFLAGS = $(subst ','\\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK):$(USE_ASCIIDOCTOR_MANPAGE))\n \n GIT-ASCIIDOCFLAGS: FORCE\n \t@FLAGS='$(TRACK_ASCIIDOCFLAGS)'; \\\n@@ -367,9 +373,16 @@ $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf asciidoctor-extensions.rb GIT-AS\n manpage-base-url.xsl: manpage-base-url.xsl.in\n \t$(QUIET_GEN)sed \"s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|\" $< > $@\n \n+ifdef TXT_TO_MAN\n+%.1 %.5 %.7 : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n+\t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\n+\t$(TXT_TO_MAN) -o $@+ $< && \\\n+\tmv $@+ $@\n+else\n %.1 %.5 %.7 : %.xml manpage-base-url.xsl $(wildcard manpage*.xsl)\n \t$(QUIET_XMLTO)$(RM) $@ && \\\n \t$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<\n+endif\n \n %.xml : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS\n \t$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \\\ndiff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\nindex d906a00803..620b3d7a88 100644\n--- a/Documentation/asciidoctor-extensions.rb\n+++ b/Documentation/asciidoctor-extensions.rb\n@@ -15,6 +15,8 @@ module Git\n \"#{target}(#{attrs[1]})\"\n elsif parent.document.basebackend? 'html'\n %(#{target}(#{attrs[1]}))\n+ elsif parent.document.basebackend? 'manpage'\n+ %(\\e\\\\fB#{target}\\e\\\\fP\\e\\\\fR(#{attrs[1]})\\e\\\\fP)\n elsif parent.document.basebackend? 'docbook'\n \"\\n\" \\\n \"#{target}\" \\\ndiff --git a/Makefile b/Makefile\nindex 93664d6714..e499152ba2 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -285,6 +285,10 @@ all::\n # Define USE_ASCIIDOCTOR to use Asciidoctor instead of AsciiDoc to build the\n # documentation.\n #\n+# Define USE_ASCIIDOCTOR_MANPAGE to use Asciidoctor's manual page backend\n+# instead of building manual pages from DocBook (using xmlto). Has no effect\n+# unless USE_ASCIIDOCTOR is set.\n+#\n # Define ASCIIDOCTOR_EXTENSIONS_LAB to point to the location of the Asciidoctor\n # Extensions Lab if you have it available.\n #\n"},{"id":"424534","messageId":"20210514003104.94644-3-sandals@crustytoothpaste.net","threadId":"55665","inReplyTo":"20210514003104.94644-1-sandals@crustytoothpaste.net","subject":"[PATCH v2 2/2] doc: remove GNU_ROFF option","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-14T00:31:04Z","receivedAt":"2021-05-14T00:31:44Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"By default, groff converts apostrophes in troff source to Unicode\napostrophes. This is helpful and desirable when being used as a\ntypesetter, since it makes the output much cleaner and more readable,\nbut it is a problem in manual pages, since apostrophes are often used\naround shell commands and these should remain in their ASCII form for\ncompatibility with the shell.\n\nFortunately, the DocBook stylesheets contain a workaround for this case:\nthey detect the special .g number register, which is set only when using\ngroff, and they define a special macro for apostrophes based on whether\nor not it is set and use that macro to write out the proper character.\nAs a result, the DocBook stylesheets handle all cases correctly\nautomatically, whether the user is using groff or not, unlike our\nGNU_ROFF code.\n\nAdditionally, this functionality was implemented in 2010. Since nobody\nis shipping a mainstream Linux distribution with security support that\nold anymore, we can just safely assume that the user has upgraded their\nsystem in the past decade and remove the GNU_ROFF option and its\ncorresponding stylesheet altogether.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/Makefile | 8 --------\n Documentation/manpage-quote-apos.xsl | 16 ----------------\n Makefile | 4 ----\n 3 files changed, 28 deletions(-)\n delete mode 100644 Documentation/manpage-quote-apos.xsl\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 891181c0f3..19dc5a2974 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -177,14 +177,6 @@ MAN_BASE_URL = file://$(htmldir)/\n endif\n XMLTO_EXTRA += -m manpage-base-url.xsl\n \n-# If your target system uses GNU groff, it may try to render\n-# apostrophes as a \"pretty\" apostrophe using unicode. This breaks\n-# cut&paste, so you should set GNU_ROFF to force them to be ASCII\n-# apostrophes. Unfortunately does not work with non-GNU roff.\n-ifdef GNU_ROFF\n-XMLTO_EXTRA += -m manpage-quote-apos.xsl\n-endif\n-\n ifdef USE_ASCIIDOCTOR\n ASCIIDOC = asciidoctor\n ASCIIDOC_CONF =\ndiff --git a/Documentation/manpage-quote-apos.xsl b/Documentation/manpage-quote-apos.xsl\ndeleted file mode 100644\nindex aeb8839f33..0000000000\n--- a/Documentation/manpage-quote-apos.xsl\n+++ /dev/null\n@@ -1,16 +0,0 @@\n-\n-\n-\n-\n- \n- \n- \n- '\n- \\(aq\n- \n-\n-\n-\ndiff --git a/Makefile b/Makefile\nindex e499152ba2..f186fd4753 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -278,10 +278,6 @@ all::\n # Define NO_ST_BLOCKS_IN_STRUCT_STAT if your platform does not have st_blocks\n # field that counts the on-disk footprint in 512-byte blocks.\n #\n-# Define GNU_ROFF if your target system uses GNU groff. This forces\n-# apostrophes to be ASCII so that cut&pasting examples to the shell\n-# will work.\n-#\n # Define USE_ASCIIDOCTOR to use Asciidoctor instead of AsciiDoc to build the\n # documentation.\n #\n"},{"id":"424539","messageId":"xmqqbl9e7yqz.fsf@gitster.g","threadId":"55665","inReplyTo":"20210514003104.94644-2-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-14T03:58:12Z","receivedAt":"2021-05-14T03:58:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"> diff --git a/Documentation/Makefile b/Documentation/Makefile\n> index 2aae4c9cbb..891181c0f3 100644\n> --- a/Documentation/Makefile\n> +++ b/Documentation/Makefile\n> @@ -192,10 +192,16 @@ ASCIIDOC_HTML = xhtml5\n> ASCIIDOC_DOCBOOK = docbook5\n> ASCIIDOC_EXTRA += -acompat-mode -atabsize=8\n> ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions\n> -ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n> +TXT_TO_HTML += -alitdd='&\\#x2d;&\\#x2d;'\n> +TXT_TO_XML += -alitdd='&\\#x2d;&\\#x2d;'\n> DBLATEX_COMMON =\n> XMLTO_EXTRA += --skip-validation\n> XMLTO_EXTRA += -x manpage.xsl\n> +ifdef USE_ASCIIDOCTOR_MANPAGE\n> +TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage\n> +TXT_TO_MAN += -aplus='+'\n> +TXT_TO_MAN += -alitdd='\\--'\n> +endif\n> endif\n\nThis hunk is wholly inside USE_ASCIIDOCTOR and deals with {litdd}\nand {plus}, which are defined in asciidoc.conf that is not read by\nAsciidoctor, so we'd need to be careful to keep these three places\n(i.e. TXT_TO_HTML, TXT_TO_XML and TXT_TO_MAN) in sync with the\nasciidoct.conf file.\n\nIt is curious that {plus} for Asciidoctor is deffined only for\nmanpages and HTML/XML side lacks the definition. Intended?\n\nIt seems that the latter has several more \"attributes\" defined that\nwe do not replicate in the Makefile---I wonder if that is a sign\nthat we can get rid of entries for asterisk, caret, startsb,\netc. from asciidoc.conf?\n\nThanks.\n"},{"id":"424542","messageId":"YJ4KLfd16qwnRqUx@coredump.intra.peff.net","threadId":"55665","inReplyTo":"xmqqbl9e7yqz.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-05-14T05:27:09Z","receivedAt":"2021-05-14T05:27:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 14, 2021 at 12:58:12PM +0900, Junio C Hamano wrote:\n\n> > diff --git a/Documentation/Makefile b/Documentation/Makefile\n> > index 2aae4c9cbb..891181c0f3 100644\n> > --- a/Documentation/Makefile\n> > +++ b/Documentation/Makefile\n> > @@ -192,10 +192,16 @@ ASCIIDOC_HTML = xhtml5\n> > ASCIIDOC_DOCBOOK = docbook5\n> > ASCIIDOC_EXTRA += -acompat-mode -atabsize=8\n> > ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions\n> > -ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n> > +TXT_TO_HTML += -alitdd='&\\#x2d;&\\#x2d;'\n> > +TXT_TO_XML += -alitdd='&\\#x2d;&\\#x2d;'\n> > DBLATEX_COMMON =\n> > XMLTO_EXTRA += --skip-validation\n> > XMLTO_EXTRA += -x manpage.xsl\n> > +ifdef USE_ASCIIDOCTOR_MANPAGE\n> > +TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage\n> > +TXT_TO_MAN += -aplus='+'\n> > +TXT_TO_MAN += -alitdd='\\--'\n> > +endif\n> > endif\n> \n> This hunk is wholly inside USE_ASCIIDOCTOR and deals with {litdd}\n> and {plus}, which are defined in asciidoc.conf that is not read by\n> Asciidoctor, so we'd need to be careful to keep these three places\n> (i.e. TXT_TO_HTML, TXT_TO_XML and TXT_TO_MAN) in sync with the\n> asciidoct.conf file.\n> \n> It is curious that {plus} for Asciidoctor is deffined only for\n> manpages and HTML/XML side lacks the definition. Intended?\n\nI don't think it's needed on the HTML/XML side, as AsciiDoctor ships\nwith reasonable conversions there to HTML entities. It's only for the\ndirect-to-manpage path that needs them. This is probably a bug in\nAsciiDoctor, but I haven't investigated fully.\n\n> It seems that the latter has several more \"attributes\" defined that\n> we do not replicate in the Makefile---I wonder if that is a sign\n> that we can get rid of entries for asterisk, caret, startsb,\n> etc. from asciidoc.conf?\n\nI don't think so. Even though AsciiDoctor ships with sensible values for\nthose attributes, AsciiDoc doesn't seem to. Try committing something\nlike this and then running \"./doc-diff HEAD^ HEAD\", which shows all\nkinds of problems.\n\n-- >8 --\nSubject: drop maybe unused attributes\n\n---\n Documentation/asciidoc.conf | 8 --------\n 1 file changed, 8 deletions(-)\n\ndiff --git a/Documentation/asciidoc.conf b/Documentation/asciidoc.conf\nindex 3e4c13971b..75f01e2fa1 100644\n--- a/Documentation/asciidoc.conf\n+++ b/Documentation/asciidoc.conf\n@@ -11,15 +11,7 @@\n (?su)[\\\\]?(?Plinkgit):(?P\\S*?)\\[(?P.*?)\\]=\n \n [attributes]\n-asterisk=*\n plus=+\n-caret=^\n-startsb=[\n-endsb=]\n-backslash=\\n-tilde=~\n-apostrophe='\n-backtick=`\n litdd=--\n \n ifdef::backend-docbook[]\n-- \n2.31.1.1062.g531ce38507\n\n"},{"id":"424581","messageId":"609e73dcbe525_431272081e@natae.notmuch","threadId":"55665","inReplyTo":"YJ21Qfnk0tpPgPqu@camp.crustytoothpaste.net","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T12:58:04Z","receivedAt":"2021-05-14T12:58:11Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"brian m. carlson wrote:\n> On 2021-05-12 at 02:48:59, Bagas Sanjaya wrote:\n> > Maybe when distros upgraded shipped Asciidoctor version to 2.0, we can\n> > bump the version requirement.\n> \n> My general policy, which need not be Git's policy (but I think is\n> reasonable), is that I will support the previous version of Debian and\n> Ubuntu LTS for a year after the new one comes out. Under that policy,\n> we'd wait until a year after Debian 11 (bullseye) is released.\n\nUnder that policy the supported version would be Debian 10 (buster),\nwhich ships with Ruby 2.5. It's more than capable of running\nasciidoctor.\n\nThe CI of asciidoctor tests versionof Ruby as old as 2.3, so Debian 10\nis safe.\n\nIn fact, I would bet you that asciidoctor works fine in Ruby 2.1 shipped\nwith Debian 8 (jessie) released in 2015. Maybe users of Debian 7 would\nhave trouble... *maybe*... It's hard to tell because Debian doesn't\neven provide package information about that release.\n\n> > > diff --git a/Makefile b/Makefile\n> > > index 93664d6714..cb75dec314 100644\n> > > --- a/Makefile\n> > > +++ b/Makefile\n> > > @@ -285,6 +285,9 @@ all::\n> > > # Define USE_ASCIIDOCTOR to use Asciidoctor instead of AsciiDoc to build the\n> > > # documentation.\n> > > #\n> > > +# Define USE_ASCIIDOCTOR_MANPAGE to use Asciidoctor's manual page backend\n> > > +# instead of building manual pages from DocBook.\n> > > +#\n> > The wording should be \"...instead of building manual pages from DocBook with\n> > xmlto\".\n> \n> I can make that change. We're not using DocBook either way, with xmlto\n> or other tooling (e.g., a plain xsltproc), so what we have here is\n> accurate.\n\nHmm...\n\ncat Documentation/manpage.xsl\n\n \n\nThat's a deliverable of the DocBook project, is it not?\n\n-- \nFelipe Contreras\n"},{"id":"424589","messageId":"CAN0heSpQz3mAe6vS__rKSfj8wCh+H3pRqauQA5TpjZkYhMBGrA@mail.gmail.com","threadId":"55665","inReplyTo":"YJt1/DO1cXNTRNxK@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2021-05-14T15:34:54Z","receivedAt":"2021-05-14T15:35:11Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Wed, 12 May 2021 at 08:30, Jeff King wrote:\n>\n> On Wed, May 12, 2021 at 02:22:06AM -0400, Jeff King wrote:\n>\n> > With that change, plus a patch I'll send in a minute, it's easy to run\n> > doc-diff on the result.\n>\n> Feel free to add it to your series if it helps (I had originally thought\n> it would just be a one-off to look at the output, but there are enough\n> changes, both correctness and style, that it may be useful to have this\n> option around).\n\nI agree that this would be useful to have longer-term.\n\n> -- >8 --\n> Subject: [PATCH] doc-diff: support --asciidoctor-direct mode\n\nFWIW: Reviewed-by: Martin Ågren \n"},{"id":"424611","messageId":"609ec960cc2f8_43127208a4@natae.notmuch","threadId":"55665","inReplyTo":"YJ24WIpnwE4LBOMt@camp.crustytoothpaste.net","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T19:02:56Z","receivedAt":"2021-05-14T19:03:07Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"brian m. carlson wrote:\n> On 2021-05-12 at 04:41:41, Felipe Contreras wrote:\n> > brian m. carlson wrote:\n> > > since some common distros are > still on 1.5.\n> > \n> > Are \"some common distros\" namely Debian stable *exclusively*?\n> > \n> > If so, I would consider flipping the default the other way around,\n> > espececially since it's only te default shipped by the Debian stable\n> > packages (easily fixed by `gem install asciidoctor`).\n> \n> CentOS 7 and Ubuntu 18.04 also ship 1.5. CentOS 8 does not appear to\n> ship Asciidoctor at all.\n\nI does not matter what version of asciidoctor they *ship* with.... At all.\n\nI happen to have accesss to an Ubuntu 18.04 machine...\n\n time gem install --user-install asciidoctor\n real\t0m3.179s\n\nIt took me 3 seconds to get asciidoctor-2.0.15.\n\nUbuntu 18.04 ships with ruby 2.5.1p57 (2018-03-29 revision 63029). And\nthat's all you need.\n\n> > > If users are using a more modern toolchain or don't care\n> > > about the rendering issues, they can enable the option.\n> > \n> > The other way around: if users are using an ancient distribution they\n> > can disable the option.\n> \n> Debian stable is not ancient. It is less than two years old, which for\n> a Linux distro or any operating system distribution, is not at all\n> considered even reasonably old.\n\nI guess your definition of what's \"old\" and mine are *very* different.\n\nBut the problem doesn't come from when the distribution was released,\nbut when the packages *inside* the distribution were released.\n\n> I am not going to propose or give my approval to a change that causes\n> problems building Git with the distro packages on Debian stable or the\n> latest Ubuntu LTS, in any way, shape, or form. People should be able to\n> use the distro packages if that's most convenient.\n\nMy proposed changes do not cause any problems building Git with any\ndistro package on Debian stable or the latest Ubuntu LTS in any way,\nshape or form.\n\n> > > Suggested-by: Bagas Sanjaya \n> > > Signed-off-by: Felipe Contreras \n> > > Signed-off-by: brian m. carlson \n> > \n> > Commit-message-by: brian m. carlson \n> > \n> > I certainly would not want to pretend to have written the text above.\n> \n> I'll reroll with the author reset.\n\nThats is not what I just requested.\n\n> > > diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb\n> > > index d906a00803..40fa87b121 100644\n> > > --- a/Documentation/asciidoctor-extensions.rb\n> > > +++ b/Documentation/asciidoctor-extensions.rb\n> > > @@ -15,6 +15,8 @@ module Git\n> > > \"#{target}(#{attrs[1]})\"\n> > > elsif parent.document.basebackend? 'html'\n> > > %(#{target}(#{attrs[1]}))\n> > > + elsif parent.document.basebackend? 'manpage'\n> > > + %(\\\\fB#{target}\\\\fP\\\\fR(#{attrs[1]})\\\\fP)\n> > \n> > I still prefer my original version, especially since:\n> > \n> > 1. I suspect most git developers are familiar with printf directives:\n> > %s.\n> > 2. Where is that \\\\fP coming from? I don't see that with xmlto, nor the\n> > publicly genrated man pages[1].\n> \n> That's coming from my knowledge of troff, having used it extensively\n> over the years, and my general hesitance to affect global state.\n\nGood. Send it as a *separate* patch.\n\nMost people in the mailing list are trying to **minimize** the diff\nbetween asciidoc+docbook and asciidoctor, not piggyback improvements.\n\nIf you want to flex your troff knowledge do it as a separate patch,\nwithout my authorship, or s-o-b line.\n\n> > 3. Doesn't work on my machine without my original \\e; I see\n> > \"\\fBgittutorial\\fR(7)\".\n> \n> Works just fine on my system using Asciidoctor 2.0.12. The \\e would\n> insert an additional escape character and shouldn't be needed unless\n> we're in copy mode (which we should not be here).\n\nI fail to see how that could work since asciidoctor replaces '\\'\nwith '\\(rs'.\n\nThis is a simple test that reproduces the issue:\n\n ruby -r asciidoctor <<\\EOF | groff -Tascii -man | less\n puts Asciidoctor.convert(\"This is a \\\\fBtest\\\\fR.\", backend: :manpage)\n EOF\n\nThose that work in your machine?\n\nThis happens both with v2.0.12 and the latest master (2.0.16.dev).\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"424612","messageId":"609eca5a42be2_43127208f2@natae.notmuch","threadId":"55665","inReplyTo":"20210514003104.94644-1-sandals@crustytoothpaste.net","subject":"RE: [PATCH v2 0/2] Asciidoctor native manpage builds","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T19:07:06Z","receivedAt":"2021-05-14T19:07:10Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"brian m. carlson wrote:\n> @@ Documentation/asciidoctor-extensions.rb: module Git\n> elsif parent.document.basebackend? 'html'\n> %(#{target}(#{attrs[1]}))\n> + elsif parent.document.basebackend? 'manpage'\n> -+ %(\\\\fB#{target}\\\\fP\\\\fR(#{attrs[1]})\\\\fP)\n> ++ %(\\e\\\\fB#{target}\\e\\\\fP\\e\\\\fR(#{attrs[1]})\\e\\\\fP)\n> elsif parent.document.basebackend? 'docbook'\n> \"\\n\" \\\n> \"#{target}\" \\\n\nHuh? Didn't you say \\e was not needed?\n\nYou are doing basically the same thing thing my patches now, except in a\nmore convoluted way.\n\n-- \nFelipe Contreras\n"},{"id":"424614","messageId":"609ed529e2306_431272087@natae.notmuch","threadId":"55665","inReplyTo":"20210514003104.94644-2-sandals@crustytoothpaste.net","subject":"RE: [PATCH v2 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T19:53:13Z","receivedAt":"2021-05-14T19:53:18Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"brian m. carlson wrote:\n> Asciidoctor contains a converter to generate man pages. In some\n> environments, where building only the manual pages and not the other\n> documentation is desired, installing a toolchain for building\n> DocBook-based manual pages may be burdensome, and using Asciidoctor\n> directly may be easier, so let's add an option to build manual pages\n> using Asciidoctor without the DocBook toolchain.\n> \n> We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't\n> contain proper handling of the apostrophe, which is controlled normally\n> by the GNU_ROFF option. This option for the DocBook toolchain, as well\n> as newer versions of Asciidoctor, makes groff output an ASCII apostrophe\n> instead of a Unicode apostrophe in text, so as to make copy and pasting\n> commands easier. These newer versions of Asciidoctor (1.5.3 and above)\n> detect groff and do the right thing in all cases, so the GNU_ROFF option\n> is obsolete in this case.\n\nI don't see what that paragraph has to do with the patch below.\n\n> We also need to update the code that tells Asciidoctor how to format our\n> linkgit macros so that it can output proper code for man pages.\n\nYes, but why shove it in this patch? Now this is is doing *two*\nlogically-independent changes.\n\n> Be careful to reset the font to the previous after the change.\n\nThis is a third change, since the current man pages already don't do\nthis:\n\n % zcat /usr/share/man/man1/git-add.1.gz | grep '\\fB'\n you must use the \\fBadd\\fR command\n\n> We insert \\e before each font-change backslash so Asciidoctor doesn't\n> convert them into \\*(rs, the reverse solidus character, and instead\n> leaves them as we wanted them.\n\nRight. So my patch was correct: it is neecessary.\n\n> Additionally, we don't want to use XML-style escapes for the litdd and\n> plus macros, so let's only use the XML-style escapes in HTML and XML and\n> use something different for our man pages.\n\nThat's a fourth change now, and one that complicates the Makefile even\nmore, when I've been trying to simplify it.\n\n> Because Asciidoctor versions before 2.0 had a few problems with man page\n> output, let's default this to off for now, since some common distros are\n> still on 1.5.\n\nCan you point what problems are those? I did a doc-diff with my patches\non asciidoctor 1.5.8 and I did not see any major problems.\n\n> If users are using a more modern toolchain or don't care\n> about the rendering issues, they can enable the option.\n\nWhat rendering issues?\n\nAlso, the many should not suffer because of the few.\n\nIf a few people doing USE_ASCIIDOCTOR=YesPlease have issues (because of\nancient packages in their distribution, and their reluctance to type\n`gem install`), then *they* can disable USE_ASCIIDOCTOR_MANPAGE (or just\ndisable USE_ASCIIDOCTOR altogether). Most people doing\nUSE_ASCIIDOCTOR=YesPlease should not suffer because of a\nminority.\n\n> Signed-off-by: Felipe Contreras \n\nI most definitely do not sign off this.\n\n-- \nFelipe Contreras\n"},{"id":"424615","messageId":"YJ7VwbELmik+RVCu@camp.crustytoothpaste.net","threadId":"55665","inReplyTo":"xmqqbl9e7yqz.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-14T19:55:45Z","receivedAt":"2021-05-14T19:55:52Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-05-14 at 03:58:12, Junio C Hamano wrote:\n> It is curious that {plus} for Asciidoctor is deffined only for\n> manpages and HTML/XML side lacks the definition. Intended?\n\nYes, that's intended, because the behavior is already correct there.\n\n> It seems that the latter has several more \"attributes\" defined that\n> we do not replicate in the Makefile---I wonder if that is a sign\n> that we can get rid of entries for asterisk, caret, startsb,\n> etc. from asciidoc.conf?\n\nI can't speak to the Python implementation, but maybe someone else can.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"424616","messageId":"609ed62a3ad1d_4312720825@natae.notmuch","threadId":"55665","inReplyTo":"xmqqbl9e7yqz.fsf@gitster.g","subject":"Re: [PATCH v2 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T19:57:30Z","receivedAt":"2021-05-14T19:57:43Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> > diff --git a/Documentation/Makefile b/Documentation/Makefile\n> > index 2aae4c9cbb..891181c0f3 100644\n> > --- a/Documentation/Makefile\n> > +++ b/Documentation/Makefile\n> > @@ -192,10 +192,16 @@ ASCIIDOC_HTML = xhtml5\n> > ASCIIDOC_DOCBOOK = docbook5\n> > ASCIIDOC_EXTRA += -acompat-mode -atabsize=8\n> > ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions\n> > -ASCIIDOC_EXTRA += -alitdd='&\\#x2d;&\\#x2d;'\n> > +TXT_TO_HTML += -alitdd='&\\#x2d;&\\#x2d;'\n> > +TXT_TO_XML += -alitdd='&\\#x2d;&\\#x2d;'\n> > DBLATEX_COMMON =\n> > XMLTO_EXTRA += --skip-validation\n> > XMLTO_EXTRA += -x manpage.xsl\n> > +ifdef USE_ASCIIDOCTOR_MANPAGE\n> > +TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage\n> > +TXT_TO_MAN += -aplus='+'\n> > +TXT_TO_MAN += -alitdd='\\--'\n> > +endif\n> > endif\n> \n> This hunk is wholly inside USE_ASCIIDOCTOR and deals with {litdd}\n> and {plus}, which are defined in asciidoc.conf that is not read by\n> Asciidoctor, so we'd need to be careful to keep these three places\n> (i.e. TXT_TO_HTML, TXT_TO_XML and TXT_TO_MAN) in sync with the\n> asciidoct.conf file.\n> \n> It is curious that {plus} for Asciidoctor is deffined only for\n> manpages and HTML/XML side lacks the definition. Intended?\n\nYes. It is a temporary workaround for a bug in asciidoctor. Eventually\nwe don't want to do this.\n\nIt's much more clearer in my patch, that contains the hack to a single\nplace inside asciidoctor-extensions.rb [1].\n\n[1] https://lore.kernel.org/git/20210514121435.504423-8-felipe.contreras@gmail.com/T/#u\n\n-- \nFelipe Contreras\n"},{"id":"424617","messageId":"YJ7W4YrboXQhMh/B@camp.crustytoothpaste.net","threadId":"55665","inReplyTo":"609eca5a42be2_43127208f2@natae.notmuch","subject":"Re: [PATCH v2 0/2] Asciidoctor native manpage builds","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-14T20:00:33Z","receivedAt":"2021-05-14T20:00:39Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-05-14 at 19:07:06, Felipe Contreras wrote:\n> brian m. carlson wrote:\n> > @@ Documentation/asciidoctor-extensions.rb: module Git\n> > elsif parent.document.basebackend? 'html'\n> > %(#{target}(#{attrs[1]}))\n> > + elsif parent.document.basebackend? 'manpage'\n> > -+ %(\\\\fB#{target}\\\\fP\\\\fR(#{attrs[1]})\\\\fP)\n> > ++ %(\\e\\\\fB#{target}\\e\\\\fP\\e\\\\fR(#{attrs[1]})\\e\\\\fP)\n> > elsif parent.document.basebackend? 'docbook'\n> > \"\\n\" \\\n> > \"#{target}\" \\\n> \n> Huh? Didn't you say \\e was not needed?\n\nYes, but I believe in that case my build was broken and I was incorrect.\n\n> You are doing basically the same thing thing my patches now, except in a\n> more convoluted way.\n\nThe way your patches do it, if someone adds a line like this:\n\n _abc linkgit:git-update-index[1] def_\n\nthe latter part (def) is not italicized. In my version, it is, which is\nthe correct behavior.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"424618","messageId":"609ed6e4e5a26_43127208b2@natae.notmuch","threadId":"55665","inReplyTo":"YJ4KLfd16qwnRqUx@coredump.intra.peff.net","subject":"Re: [PATCH v2 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T20:00:36Z","receivedAt":"2021-05-14T20:00:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Jeff King wrote:\n> > It is curious that {plus} for Asciidoctor is deffined only for\n> > manpages and HTML/XML side lacks the definition. Intended?\n> \n> I don't think it's needed on the HTML/XML side, as AsciiDoctor ships\n> with reasonable conversions there to HTML entities. It's only for the\n> direct-to-manpage path that needs them. This is probably a bug in\n> AsciiDoctor, but I haven't investigated fully.\n\nI have, as I have explained in my patch series, which does isolate this\nworkaround in a single patch [1].\n\n[1] https://lore.kernel.org/git/20210514121435.504423-8-felipe.contreras@gmail.com/T/#u\n\n-- \nFelipe Contreras\n"},{"id":"424620","messageId":"YJ7avgek24eu+HHM@camp.crustytoothpaste.net","threadId":"55665","inReplyTo":"609ed529e2306_431272087@natae.notmuch","subject":"Re: [PATCH v2 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-05-14T20:17:02Z","receivedAt":"2021-05-14T20:17:09Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-05-14 at 19:53:13, Felipe Contreras wrote:\n> brian m. carlson wrote:\n> > Asciidoctor contains a converter to generate man pages. In some\n> > environments, where building only the manual pages and not the other\n> > documentation is desired, installing a toolchain for building\n> > DocBook-based manual pages may be burdensome, and using Asciidoctor\n> > directly may be easier, so let's add an option to build manual pages\n> > using Asciidoctor without the DocBook toolchain.\n> > \n> > We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't\n> > contain proper handling of the apostrophe, which is controlled normally\n> > by the GNU_ROFF option. This option for the DocBook toolchain, as well\n> > as newer versions of Asciidoctor, makes groff output an ASCII apostrophe\n> > instead of a Unicode apostrophe in text, so as to make copy and pasting\n> > commands easier. These newer versions of Asciidoctor (1.5.3 and above)\n> > detect groff and do the right thing in all cases, so the GNU_ROFF option\n> > is obsolete in this case.\n> \n> I don't see what that paragraph has to do with the patch below.\n\nIt's relevant because it explains why it's acceptable to discount that\nfeature that we're not supporting as part of the patch.\n\n> > We also need to update the code that tells Asciidoctor how to format our\n> > linkgit macros so that it can output proper code for man pages.\n> \n> Yes, but why shove it in this patch? Now this is is doing *two*\n> logically-independent changes.\n\nThis is one logical change: implementing Asciidoctor native support for\nman pages.\n\n> > Be careful to reset the font to the previous after the change.\n> \n> This is a third change, since the current man pages already don't do\n> this:\n> \n> % zcat /usr/share/man/man1/git-add.1.gz | grep '\\fB'\n> you must use the \\fBadd\\fR command\n\nAs explained downthread, we don't know in the manual pages what font\nstyling we're in. troff has font-change commands, not nesting begin-end\npairs, for italics and bold. If the linkgit macro appears in the middle\nof a passage in italics, by not using \\fP, we'll force the rest of the\ntext which is to be italicized into roman.\n\nThe toolchain, whether Asciidoctor or the XSLT stylesheets, _does_ have\nthis context and therefore can explicitly move between bold and roman,\nbut our extensions do not.\n\n> > We insert \\e before each font-change backslash so Asciidoctor doesn't\n> > convert them into \\*(rs, the reverse solidus character, and instead\n> > leaves them as we wanted them.\n> \n> Right. So my patch was correct: it is neecessary.\n\nYes, I agree that it's necessary. As I stated downthread, it appears my\nbranch was in a broken state.\n\n> > Additionally, we don't want to use XML-style escapes for the litdd and\n> > plus macros, so let's only use the XML-style escapes in HTML and XML and\n> > use something different for our man pages.\n> \n> That's a fourth change now, and one that complicates the Makefile even\n> more, when I've been trying to simplify it.\n\nI'm sorry that this complicates work you'd like to do, but\nunfortunately, the other option is broken rendering.\n\n> > If users are using a more modern toolchain or don't care\n> > about the rendering issues, they can enable the option.\n> \n> What rendering issues?\n\nThey were mentioned upthread.\n\n> Also, the many should not suffer because of the few.\n> \n> If a few people doing USE_ASCIIDOCTOR=YesPlease have issues (because of\n> ancient packages in their distribution, and their reluctance to type\n> `gem install`), then *they* can disable USE_ASCIIDOCTOR_MANPAGE (or just\n> disable USE_ASCIIDOCTOR altogether). Most people doing\n> USE_ASCIIDOCTOR=YesPlease should not suffer because of a\n> minority.\n\nI don't believe we're going to agree on this. I believe we should\nchoose defaults that work with the most popular Linux distributions, and\nyou don't. I think your approach is unnecessarily hostile to ordinary\nusers and developers and understates the value that people derive from\ndistributions.\n\n> > Signed-off-by: Felipe Contreras \n> \n> I most definitely do not sign off this.\n\nThis sign-off is not an approval of the patch. This is the statement\nthat this patch is based on your work which is subject to the license\nspecified in the file and that you agreed that your original\ncontribution (now modified) could be distributed with your sign-off. In\nno portion of the DCO does it state that a sign-off means you think the\npatch is a good idea or that you approve of it in any way.\n\nI do want to be clear that I'm aware you don't approve of this patch and\nthat's why I submitted a counterproposal: because I don't approve of\nyour patch and you seem unwilling to make changes to it. I would love\nnothing more than to remove your name from it entirely, but\nunfortunately, that's not possible with the DCO.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"424623","messageId":"609ee3246708a_43127208a6@natae.notmuch","threadId":"55665","inReplyTo":"YJ7VwbELmik+RVCu@camp.crustytoothpaste.net","subject":"Re: [PATCH v2 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T20:52:52Z","receivedAt":"2021-05-14T20:52:56Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"brian m. carlson wrote:\n> On 2021-05-14 at 03:58:12, Junio C Hamano wrote:\n> > It seems that the latter has several more \"attributes\" defined that\n> > we do not replicate in the Makefile---I wonder if that is a sign\n> > that we can get rid of entries for asterisk, caret, startsb,\n> > etc. from asciidoc.conf?\n> \n> I can't speak to the Python implementation, but maybe someone else can.\n\nLooking at the code there's a global configuration file\n(/etc/asciidoc/asciidoc.conf on my system), but the only attribute they\ndefine the same way as git is `plus`, so that's the only one we could\ndrop.\n\n-- \nFelipe Contreras\n"},{"id":"424625","messageId":"609ee9d117baa_4312720826@natae.notmuch","threadId":"55665","inReplyTo":"YJ7W4YrboXQhMh/B@camp.crustytoothpaste.net","subject":"Re: [PATCH v2 0/2] Asciidoctor native manpage builds","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T21:21:21Z","receivedAt":"2021-05-14T21:21:24Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"brian m. carlson wrote:\n> On 2021-05-14 at 19:07:06, Felipe Contreras wrote:\n> > brian m. carlson wrote:\n> > > @@ Documentation/asciidoctor-extensions.rb: module Git\n> > > elsif parent.document.basebackend? 'html'\n> > > %(#{target}(#{attrs[1]}))\n> > > + elsif parent.document.basebackend? 'manpage'\n> > > -+ %(\\\\fB#{target}\\\\fP\\\\fR(#{attrs[1]})\\\\fP)\n> > > ++ %(\\e\\\\fB#{target}\\e\\\\fP\\e\\\\fR(#{attrs[1]})\\e\\\\fP)\n> > > elsif parent.document.basebackend? 'docbook'\n> > > \"\\n\" \\\n> > > \"#{target}\" \\\n> > \n> > Huh? Didn't you say \\e was not needed?\n> \n> Yes, but I believe in that case my build was broken and I was incorrect.\n\nI see. If it helps you I'm using this script [1] to run the specific\nversion of asciidoctor of a git repository (their wrapper is wrong).\n\n> > You are doing basically the same thing thing my patches now, except in a\n> > more convoluted way.\n> \n> The way your patches do it, if someone adds a line like this:\n> \n> _abc linkgit:git-update-index[1] def_\n> \n> the latter part (def) is not italicized. In my version, it is, which is\n> the correct behavior.\n\nRight, but my version is precisely what asciidoc+docbook generates in\nthe simplest case.\n\nI agree your version is superior (although I wouldn't do the second\n\\fR \\fP). But that belongs in a separate patch IMO.\n\nCheers.\n\n[1] https://dpaste.com/3AEDTFCSK\n\n-- \nFelipe Contreras\n"},{"id":"424628","messageId":"609f0840eb383_4312720868@natae.notmuch","threadId":"55665","inReplyTo":"YJ7avgek24eu+HHM@camp.crustytoothpaste.net","subject":"Re: [PATCH v2 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-14T23:31:12Z","receivedAt":"2021-05-14T23:31:17Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"brian m. carlson wrote:\n> On 2021-05-14 at 19:53:13, Felipe Contreras wrote:\n> > brian m. carlson wrote:\n> > > We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't\n> > > contain proper handling of the apostrophe, which is controlled normally\n> > > by the GNU_ROFF option. This option for the DocBook toolchain, as well\n> > > as newer versions of Asciidoctor, makes groff output an ASCII apostrophe\n> > > instead of a Unicode apostrophe in text, so as to make copy and pasting\n> > > commands easier. These newer versions of Asciidoctor (1.5.3 and above)\n> > > detect groff and do the right thing in all cases, so the GNU_ROFF option\n> > > is obsolete in this case.\n> > \n> > I don't see what that paragraph has to do with the patch below.\n> \n> It's relevant because it explains why it's acceptable to discount that\n> feature that we're not supporting as part of the patch.\n\nIt's easier to first drop that feature, and then you don't have to\nexplain anything.\n\n> > > We also need to update the code that tells Asciidoctor how to format our\n> > > linkgit macros so that it can output proper code for man pages.\n> > \n> > Yes, but why shove it in this patch? Now this is is doing *two*\n> > logically-independent changes.\n> \n> This is one logical change: implementing Asciidoctor native support for\n> man pages.\n\nBy that logic all patch series can be a \"logical change\": \"implement\nSHA-256 support\".\n\n> > > Be careful to reset the font to the previous after the change.\n> > \n> > This is a third change, since the current man pages already don't do\n> > this:\n> > \n> > % zcat /usr/share/man/man1/git-add.1.gz | grep '\\fB'\n> > you must use the \\fBadd\\fR command\n> \n> As explained downthread, we don't know in the manual pages what font\n> styling we're in. troff has font-change commands, not nesting begin-end\n> pairs, for italics and bold. If the linkgit macro appears in the middle\n> of a passage in italics, by not using \\fP, we'll force the rest of the\n> text which is to be italicized into roman.\n> \n> The toolchain, whether Asciidoctor or the XSLT stylesheets, _does_ have\n> this context and therefore can explicitly move between bold and roman,\n> but our extensions do not.\n\nIndeed but it's rare (there's probably zero instances), and it increases\nthe delta. Yes, it's more correct, but it trades a hypothetical benefit\nfor a real disadvantage.\n\nEither way I see no point in arguing about this. If you feel strongly\nabout this I can include it in my version.\n\n> > > Additionally, we don't want to use XML-style escapes for the litdd and\n> > > plus macros, so let's only use the XML-style escapes in HTML and XML and\n> > > use something different for our man pages.\n> > \n> > That's a fourth change now, and one that complicates the Makefile even\n> > more, when I've been trying to simplify it.\n> \n> I'm sorry that this complicates work you'd like to do, but\n> unfortunately, the other option is broken rendering.\n\nClean and maintainable code is a benefit to the project, not just me.\nAnd the Makefile is code too.\n\nForget about me, a clean and simple Makefile is better than a cluttered\nand complex Makefile. That is the point.\n\nYes, the rendering is \"broken\" without the change (that's loaded\nlanguage, but OK), we want to know precisely how, and how it got fixed.\nWe don't wan to sneak in all the fixes in the world in the first patch.\n\nMoreover, there's no dicotomy here; we can fix the \"broken\" state in\nother ways that don't complicate the Makefile, as I did in my patch\nseries [1].\n\nBut in fact, I have an even simpler version now:\n\n Asciidoctor::Extensions.register do\n+ # Override attributes for man pages.\n+ # https://github.com/asciidoctor/asciidoctor/issues/4059\n+ if document.backend == 'manpage'\n+ document.attributes.merge!({ 'litdd' => '\\--', 'plus' => '+' })\n+ end\n+\n inline_macro Git::Documentation::LinkGitProcessor, :linkgit\n postprocessor Git::Documentation::DocumentPostProcessor\n end\n\n> > > If users are using a more modern toolchain or don't care\n> > > about the rendering issues, they can enable the option.\n> > \n> > What rendering issues?\n> \n> They were mentioned upthread.\n\nNo. Pepole mentioned issues they *think* existed, nobody pointed out an\nactual reproducible issue.\n\nAs you experienced with the \\e setback, the issue could have been with\ntheir build, or it could be present in v1.5.7, but not v1.5.8. It's hard\nto know if nobody spells out what the issue is.\n\nSo... What rendering issues?\n\n> > Also, the many should not suffer because of the few.\n> > \n> > If a few people doing USE_ASCIIDOCTOR=YesPlease have issues (because of\n> > ancient packages in their distribution, and their reluctance to type\n> > `gem install`), then *they* can disable USE_ASCIIDOCTOR_MANPAGE (or just\n> > disable USE_ASCIIDOCTOR altogether). Most people doing\n> > USE_ASCIIDOCTOR=YesPlease should not suffer because of a\n> > minority.\n> \n> I don't believe we're going to agree on this. I believe we should\n> choose defaults that work with the most popular Linux distributions, and\n> you don't.\n\nUntrue.\n\n`make USE_ASCIIDOCTOR= doc` works perfectly fine on Debian stable with\nmy patches, and that's the default, nobody is changing that.\n\nAnd so does this:\n\n gem install asciidoctor && make USE_ASCIIDOCTOR=1 doc\n\nBut in fact, so does `make USE_ASCIIDOCTOR=1 doc`, because asciidoctor\n1.5.8 works just fine. I just did a doc-diff with v1.5.8, and I\nchecked everything without finding any serious issue (not present in\n2.0.15).\n\nThe only issue is that \\\\ was not handled correctly, but I now have a\nworkaround for that:\n\n if document.basebackend?('manpage') and Asciidoctor::VERSION < '2.0.11'\n postprocessor do\n process do |document, output|\n output.gsub(\"\\\\(rs\\\\\\\\\", \"\\\\(rs\\\\(rs\\\\\")\n end\n end\n end\n\n> I think your approach is unnecessarily hostile to ordinary\n> users and developers and understates the value that people derive from\n> distributions.\n\nPlease tell me exactly how my patches are hostile to \"ordinary\ndevelopers\" (who are not ordinary at all).\n\n 1. Will they be able to build the documentation with default flags?\n Yes\n 2. Will they be able to build with USE_ASCIIDOCTOR=1?\n Yes\n 3. Will they be able to see a reasonable output?\n Yes\n 4. Will they be forced to install a gem?\n No\n\nSo where exactly is the hostility?\n\n> > > Signed-off-by: Felipe Contreras \n> > \n> > I most definitely do not sign off this.\n> \n> This sign-off is not an approval of the patch.\n\nYes it is. According to the Developer Certificate of Origin [2] I must:\n\n agree that a record of the contribution (including my sign-off) is\n maintained indefinitely\n\nWhich I don't.\n\nFeel free to disregard my lack of agreement, but I'm stating it for the\nrecord.\n\n> I do want to be clear that I'm aware you don't approve of this patch and\n> that's why I submitted a counterproposal: because I don't approve of\n> your patch\n\nThat happens.\n\n> and you seem unwilling to make changes to it.\n\nJust because I disagree with your changes doesn't mean I'm unwilling to\nmake changes to my patch, especially since I already agreed on making\nchanges to my patch.\n\n> I would love nothing more than to remove your name from it entirely,\n> but unfortunately, that's not possible with the DCO.\n\nIt's not possible *if* 1) you use my code, and 2) you made changes I'm\nopposed to. But you are already in violation of the DCO anyway by\ndisregarding my agreement, which is mandatory.\n\nIf you don't want to be in violation of the DCO you could try to fix\neither 1) or 2).\n\nCheers.\n\n[1] https://lore.kernel.org/git/20210514121435.504423-8-felipe.contreras@gmail.com/T/#u\n[2] https://developercertificate.org/\n\n-- \nFelipe Contreras\n"},{"id":"424656","messageId":"609fcbe4e00f8_c10bc208b2@natae.notmuch","threadId":"55665","inReplyTo":"YJ21Qfnk0tpPgPqu@camp.crustytoothpaste.net","subject":"Re: [PATCH 1/2] doc: add an option to have Asciidoctor build man pages directly","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-15T13:25:56Z","receivedAt":"2021-05-15T13:26:02Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"brian m. carlson wrote:\n> On 2021-05-12 at 02:48:59, Bagas Sanjaya wrote:\n> > On 12/05/21 09.11, brian m. carlson wrote:\n> > > We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't\n> > > contain proper handling of the apostrophe, which is controlled normally\n> > > by the GNU_ROFF option. This option for the DocBook toolchain, as well\n> > > as newer versions of Asciidoctor, makes groff output an ASCII apostrophe\n> > > instead of a Unicode apostrophe in text, so as to make copy and pasting\n> > > commands easier. These newer versions of Asciidoctor detect groff and\n> > > do the right thing in all cases, so the GNU_ROFF option is obsolete in\n> > > this case.\n> > \n> > At what version of Asciidoctor the apostrophe handling is corrected?\n> \n> The first released version is 1.5.3.\n\nI just went ahead to check that, and from the very first commit [1]\nasciidoctor generated quotes compatible with groff:\n\n git filter\\-branch \\-\\-tree\\-filter \\(aqrm filename\\(aq HEAD\n\nSo it has *always* worked.\n\nYou can see it from the code:\n\n gsub('\\'', '\\\\(aq'). # apostrophe-quote\n\nIn fact, they never changed that, so it should fail in Solaris, or\nanything that doesn't use groff.\n\nI've sent them a fix [2].\n\nWhat are these \"newever versions\" that do the right thing in all cases?\n\n[1] https://github.com/asciidoctor/asciidoctor/commit/7bddc416c92ff9d16c721b03bda7ed80c1e4c45f\n[2] https://github.com/asciidoctor/asciidoctor/pull/4060\n\n-- \nFelipe Contreras\n"},{"id":"424657","messageId":"609fccedc384_c10bc208f2@natae.notmuch","threadId":"55665","inReplyTo":"YJ3ASK/LezQ86ct1@camp.crustytoothpaste.net","subject":"Re: [PATCH 2/2] doc: remove GNU_ROFF option","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-15T13:30:21Z","receivedAt":"2021-05-15T13:30:29Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"brian m. carlson wrote:\n> On 2021-05-12 at 04:45:53, Felipe Contreras wrote:\n> > I'm not sure of all that, but my machine uses Arch Linux, it ships with\n> > groff, I've never used GNU_ROFF=1, and I can copy text with apostrophes\n> > from the genrated man pages just fine.\n> \n> I'll rephrase to be clearer. Solaris 10 is still security supported,\n> but no major Linux distro is, and I think we'll be both be fine dropping\n> support for OSes shipped in 2005.\n> \n> I'm glad to hear confirmation that things work for you, though.\n\nI took at deep-dive and it turns Arch Linux configures groff to convert\n\\' to ', so even if git was doing something wrong, I wouldn't have\nnoticed.\n\nDocbook fixed their problem in 2010, and I just sent a patch for\nasciidoctor to properly fix their code as well. It should work on groff\nthough.\n\nThe configuration is in: `/usr/share/groff/site-tmac/man.local`, if you\nwant to check what your system is doing.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}