diff mbox series

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

Message ID 20210514003104.94644-2-sandals@crustytoothpaste.net (mailing list archive)
State New
Headers show
Series Asciidoctor native manpage builds | expand

Commit Message

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

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

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

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

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

Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/Makefile                  | 17 +++++++++++++++--
 Documentation/asciidoctor-extensions.rb |  2 ++
 Makefile                                |  4 ++++
 3 files changed, 21 insertions(+), 2 deletions(-)

Comments

Junio C Hamano May 14, 2021, 3:58 a.m. UTC | #1
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 2aae4c9cbb..891181c0f3 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -192,10 +192,16 @@ ASCIIDOC_HTML = xhtml5
>  ASCIIDOC_DOCBOOK = docbook5
>  ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
>  ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
> -ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
> +TXT_TO_HTML += -alitdd='&\#x2d;&\#x2d;'
> +TXT_TO_XML  += -alitdd='&\#x2d;&\#x2d;'
>  DBLATEX_COMMON =
>  XMLTO_EXTRA += --skip-validation
>  XMLTO_EXTRA += -x manpage.xsl
> +ifdef USE_ASCIIDOCTOR_MANPAGE
> +TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage
> +TXT_TO_MAN += -aplus='+'
> +TXT_TO_MAN += -alitdd='\--'
> +endif
>  endif

This hunk is wholly inside USE_ASCIIDOCTOR and deals with {litdd}
and {plus}, which are defined in asciidoc.conf that is not read by
Asciidoctor, so we'd need to be careful to keep these three places
(i.e. TXT_TO_HTML, TXT_TO_XML and TXT_TO_MAN) in sync with the
asciidoct.conf file.

It is curious that {plus} for Asciidoctor is deffined only for
manpages and HTML/XML side lacks the definition.  Intended?

It seems that the latter has several more "attributes" defined that
we do not replicate in the Makefile---I wonder if that is a sign
that we can get rid of entries for asterisk, caret, startsb,
etc. from asciidoc.conf?

Thanks.
Jeff King May 14, 2021, 5:27 a.m. UTC | #2
On Fri, May 14, 2021 at 12:58:12PM +0900, Junio C Hamano wrote:

> > diff --git a/Documentation/Makefile b/Documentation/Makefile
> > index 2aae4c9cbb..891181c0f3 100644
> > --- a/Documentation/Makefile
> > +++ b/Documentation/Makefile
> > @@ -192,10 +192,16 @@ ASCIIDOC_HTML = xhtml5
> >  ASCIIDOC_DOCBOOK = docbook5
> >  ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
> >  ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
> > -ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
> > +TXT_TO_HTML += -alitdd='&\#x2d;&\#x2d;'
> > +TXT_TO_XML  += -alitdd='&\#x2d;&\#x2d;'
> >  DBLATEX_COMMON =
> >  XMLTO_EXTRA += --skip-validation
> >  XMLTO_EXTRA += -x manpage.xsl
> > +ifdef USE_ASCIIDOCTOR_MANPAGE
> > +TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage
> > +TXT_TO_MAN += -aplus='+'
> > +TXT_TO_MAN += -alitdd='\--'
> > +endif
> >  endif
> 
> This hunk is wholly inside USE_ASCIIDOCTOR and deals with {litdd}
> and {plus}, which are defined in asciidoc.conf that is not read by
> Asciidoctor, so we'd need to be careful to keep these three places
> (i.e. TXT_TO_HTML, TXT_TO_XML and TXT_TO_MAN) in sync with the
> asciidoct.conf file.
> 
> It is curious that {plus} for Asciidoctor is deffined only for
> manpages and HTML/XML side lacks the definition.  Intended?

I don't think it's needed on the HTML/XML side, as AsciiDoctor ships
with reasonable conversions there to HTML entities. It's only for the
direct-to-manpage path that needs them. This is probably a bug in
AsciiDoctor, but I haven't investigated fully.

> It seems that the latter has several more "attributes" defined that
> we do not replicate in the Makefile---I wonder if that is a sign
> that we can get rid of entries for asterisk, caret, startsb,
> etc. from asciidoc.conf?

I don't think so. Even though AsciiDoctor ships with sensible values for
those attributes, AsciiDoc doesn't seem to. Try committing something
like this and then running "./doc-diff HEAD^ HEAD", which shows all
kinds of problems.

-- >8 --
Subject: drop maybe unused attributes

---
 Documentation/asciidoc.conf | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/Documentation/asciidoc.conf b/Documentation/asciidoc.conf
index 3e4c13971b..75f01e2fa1 100644
--- a/Documentation/asciidoc.conf
+++ b/Documentation/asciidoc.conf
@@ -11,15 +11,7 @@
 (?su)[\\]?(?P<name>linkgit):(?P<target>\S*?)\[(?P<attrlist>.*?)\]=
 
 [attributes]
-asterisk=&#42;
 plus=&#43;
-caret=&#94;
-startsb=&#91;
-endsb=&#93;
-backslash=&#92;
-tilde=&#126;
-apostrophe=&#39;
-backtick=&#96;
 litdd=&#45;&#45;
 
 ifdef::backend-docbook[]
Felipe Contreras May 14, 2021, 7:53 p.m. UTC | #3
brian m. carlson wrote:
> Asciidoctor contains a converter to generate man pages.  In some
> environments, where building only the manual pages and not the other
> documentation is desired, installing a toolchain for building
> DocBook-based manual pages may be burdensome, and using Asciidoctor
> directly may be easier, so let's add an option to build manual pages
> using Asciidoctor without the DocBook toolchain.
> 
> We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't
> contain proper handling of the apostrophe, which is controlled normally
> by the GNU_ROFF option.  This option for the DocBook toolchain, as well
> as newer versions of Asciidoctor, makes groff output an ASCII apostrophe
> instead of a Unicode apostrophe in text, so as to make copy and pasting
> commands easier.  These newer versions of Asciidoctor (1.5.3 and above)
> detect groff and do the right thing in all cases, so the GNU_ROFF option
> is obsolete in this case.

I don't see what that paragraph has to do with the patch below.

> We also need to update the code that tells Asciidoctor how to format our
> linkgit macros so that it can output proper code for man pages.

Yes, but why shove it in this patch? Now this is is doing *two*
logically-independent changes.

> Be careful to reset the font to the previous after the change.

This is a third change, since the current man pages already don't do
this:

  % zcat /usr/share/man/man1/git-add.1.gz | grep '\fB'
  you must use the \fBadd\fR command

> We insert \e before each font-change backslash so Asciidoctor doesn't
> convert them into \*(rs, the reverse solidus character, and instead
> leaves them as we wanted them.

Right. So my patch was correct: it is neecessary.

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

That's a fourth change now, and one that complicates the Makefile even
more, when I've been trying to simplify it.

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

Can you point what problems are those? I did a doc-diff with my patches
on asciidoctor 1.5.8 and I did not see any major problems.

> If users are using a more modern toolchain or don't care
> about the rendering issues, they can enable the option.

What rendering issues?

Also, the many should not suffer because of the few.

If a few people doing USE_ASCIIDOCTOR=YesPlease have issues (because of
ancient packages in their distribution, and their reluctance to type
`gem install`), then *they* can disable USE_ASCIIDOCTOR_MANPAGE (or just
disable USE_ASCIIDOCTOR altogether). Most people doing
USE_ASCIIDOCTOR=YesPlease should not suffer because of a
minority.

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

I most definitely do not sign off this.
brian m. carlson May 14, 2021, 7:55 p.m. UTC | #4
On 2021-05-14 at 03:58:12, Junio C Hamano wrote:
> It is curious that {plus} for Asciidoctor is deffined only for
> manpages and HTML/XML side lacks the definition.  Intended?

Yes, that's intended, because the behavior is already correct there.

> It seems that the latter has several more "attributes" defined that
> we do not replicate in the Makefile---I wonder if that is a sign
> that we can get rid of entries for asterisk, caret, startsb,
> etc. from asciidoc.conf?

I can't speak to the Python implementation, but maybe someone else can.
Felipe Contreras May 14, 2021, 7:57 p.m. UTC | #5
Junio C Hamano wrote:
> > diff --git a/Documentation/Makefile b/Documentation/Makefile
> > index 2aae4c9cbb..891181c0f3 100644
> > --- a/Documentation/Makefile
> > +++ b/Documentation/Makefile
> > @@ -192,10 +192,16 @@ ASCIIDOC_HTML = xhtml5
> >  ASCIIDOC_DOCBOOK = docbook5
> >  ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
> >  ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
> > -ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
> > +TXT_TO_HTML += -alitdd='&\#x2d;&\#x2d;'
> > +TXT_TO_XML  += -alitdd='&\#x2d;&\#x2d;'
> >  DBLATEX_COMMON =
> >  XMLTO_EXTRA += --skip-validation
> >  XMLTO_EXTRA += -x manpage.xsl
> > +ifdef USE_ASCIIDOCTOR_MANPAGE
> > +TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage
> > +TXT_TO_MAN += -aplus='+'
> > +TXT_TO_MAN += -alitdd='\--'
> > +endif
> >  endif
> 
> This hunk is wholly inside USE_ASCIIDOCTOR and deals with {litdd}
> and {plus}, which are defined in asciidoc.conf that is not read by
> Asciidoctor, so we'd need to be careful to keep these three places
> (i.e. TXT_TO_HTML, TXT_TO_XML and TXT_TO_MAN) in sync with the
> asciidoct.conf file.
> 
> It is curious that {plus} for Asciidoctor is deffined only for
> manpages and HTML/XML side lacks the definition.  Intended?

Yes. It is a temporary workaround for a bug in asciidoctor. Eventually
we don't want to do this.

It's much more clearer in my patch, that contains the hack to a single
place inside asciidoctor-extensions.rb [1].

[1] https://lore.kernel.org/git/20210514121435.504423-8-felipe.contreras@gmail.com/T/#u
Felipe Contreras May 14, 2021, 8 p.m. UTC | #6
Jeff King wrote:
> > It is curious that {plus} for Asciidoctor is deffined only for
> > manpages and HTML/XML side lacks the definition.  Intended?
> 
> I don't think it's needed on the HTML/XML side, as AsciiDoctor ships
> with reasonable conversions there to HTML entities. It's only for the
> direct-to-manpage path that needs them. This is probably a bug in
> AsciiDoctor, but I haven't investigated fully.

I have, as I have explained in my patch series, which does isolate this
workaround in a single patch [1].

[1] https://lore.kernel.org/git/20210514121435.504423-8-felipe.contreras@gmail.com/T/#u
brian m. carlson May 14, 2021, 8:17 p.m. UTC | #7
On 2021-05-14 at 19:53:13, Felipe Contreras wrote:
> brian m. carlson wrote:
> > Asciidoctor contains a converter to generate man pages.  In some
> > environments, where building only the manual pages and not the other
> > documentation is desired, installing a toolchain for building
> > DocBook-based manual pages may be burdensome, and using Asciidoctor
> > directly may be easier, so let's add an option to build manual pages
> > using Asciidoctor without the DocBook toolchain.
> > 
> > We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't
> > contain proper handling of the apostrophe, which is controlled normally
> > by the GNU_ROFF option.  This option for the DocBook toolchain, as well
> > as newer versions of Asciidoctor, makes groff output an ASCII apostrophe
> > instead of a Unicode apostrophe in text, so as to make copy and pasting
> > commands easier.  These newer versions of Asciidoctor (1.5.3 and above)
> > detect groff and do the right thing in all cases, so the GNU_ROFF option
> > is obsolete in this case.
> 
> I don't see what that paragraph has to do with the patch below.

It's relevant because it explains why it's acceptable to discount that
feature that we're not supporting as part of the patch.

> > We also need to update the code that tells Asciidoctor how to format our
> > linkgit macros so that it can output proper code for man pages.
> 
> Yes, but why shove it in this patch? Now this is is doing *two*
> logically-independent changes.

This is one logical change: implementing Asciidoctor native support for
man pages.

> > Be careful to reset the font to the previous after the change.
> 
> This is a third change, since the current man pages already don't do
> this:
> 
>   % zcat /usr/share/man/man1/git-add.1.gz | grep '\fB'
>   you must use the \fBadd\fR command

As explained downthread, we don't know in the manual pages what font
styling we're in.  troff has font-change commands, not nesting begin-end
pairs, for italics and bold.  If the linkgit macro appears in the middle
of a passage in italics, by not using \fP, we'll force the rest of the
text which is to be italicized into roman.

The toolchain, whether Asciidoctor or the XSLT stylesheets, _does_ have
this context and therefore can explicitly move between bold and roman,
but our extensions do not.

> > We insert \e before each font-change backslash so Asciidoctor doesn't
> > convert them into \*(rs, the reverse solidus character, and instead
> > leaves them as we wanted them.
> 
> Right. So my patch was correct: it is neecessary.

Yes, I agree that it's necessary.  As I stated downthread, it appears my
branch was in a broken state.

> > Additionally, we don't want to use XML-style escapes for the litdd and
> > plus macros, so let's only use the XML-style escapes in HTML and XML and
> > use something different for our man pages.
> 
> That's a fourth change now, and one that complicates the Makefile even
> more, when I've been trying to simplify it.

I'm sorry that this complicates work you'd like to do, but
unfortunately, the other option is broken rendering.

> > If users are using a more modern toolchain or don't care
> > about the rendering issues, they can enable the option.
> 
> What rendering issues?

They were mentioned upthread.

> Also, the many should not suffer because of the few.
> 
> If a few people doing USE_ASCIIDOCTOR=YesPlease have issues (because of
> ancient packages in their distribution, and their reluctance to type
> `gem install`), then *they* can disable USE_ASCIIDOCTOR_MANPAGE (or just
> disable USE_ASCIIDOCTOR altogether). Most people doing
> USE_ASCIIDOCTOR=YesPlease should not suffer because of a
> minority.

I don't believe we're going to agree on this.  I believe we should
choose defaults that work with the most popular Linux distributions, and
you don't.  I think your approach is unnecessarily hostile to ordinary
users and developers and understates the value that people derive from
distributions.

> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> 
> I most definitely do not sign off this.

This sign-off is not an approval of the patch.  This is the statement
that this patch is based on your work which is subject to the license
specified in the file and that you agreed that your original
contribution (now modified) could be distributed with your sign-off.  In
no portion of the DCO does it state that a sign-off means you think the
patch is a good idea or that you approve of it in any way.

I do want to be clear that I'm aware you don't approve of this patch and
that's why I submitted a counterproposal: because I don't approve of
your patch and you seem unwilling to make changes to it.  I would love
nothing more than to remove your name from it entirely, but
unfortunately, that's not possible with the DCO.
Felipe Contreras May 14, 2021, 8:52 p.m. UTC | #8
brian m. carlson wrote:
> On 2021-05-14 at 03:58:12, Junio C Hamano wrote:
> > It seems that the latter has several more "attributes" defined that
> > we do not replicate in the Makefile---I wonder if that is a sign
> > that we can get rid of entries for asterisk, caret, startsb,
> > etc. from asciidoc.conf?
> 
> I can't speak to the Python implementation, but maybe someone else can.

Looking at the code there's a global configuration file
(/etc/asciidoc/asciidoc.conf on my system), but the only attribute they
define the same way as git is `plus`, so that's the only one we could
drop.
Felipe Contreras May 14, 2021, 11:31 p.m. UTC | #9
brian m. carlson wrote:
> On 2021-05-14 at 19:53:13, Felipe Contreras wrote:
> > brian m. carlson wrote:
> > > We generally require Asciidoctor 1.5, but versions before 1.5.3 didn't
> > > contain proper handling of the apostrophe, which is controlled normally
> > > by the GNU_ROFF option.  This option for the DocBook toolchain, as well
> > > as newer versions of Asciidoctor, makes groff output an ASCII apostrophe
> > > instead of a Unicode apostrophe in text, so as to make copy and pasting
> > > commands easier.  These newer versions of Asciidoctor (1.5.3 and above)
> > > detect groff and do the right thing in all cases, so the GNU_ROFF option
> > > is obsolete in this case.
> > 
> > I don't see what that paragraph has to do with the patch below.
> 
> It's relevant because it explains why it's acceptable to discount that
> feature that we're not supporting as part of the patch.

It's easier to first drop that feature, and then you don't have to
explain anything.

> > > We also need to update the code that tells Asciidoctor how to format our
> > > linkgit macros so that it can output proper code for man pages.
> > 
> > Yes, but why shove it in this patch? Now this is is doing *two*
> > logically-independent changes.
> 
> This is one logical change: implementing Asciidoctor native support for
> man pages.

By that logic all patch series can be a "logical change": "implement
SHA-256 support".

> > > Be careful to reset the font to the previous after the change.
> > 
> > This is a third change, since the current man pages already don't do
> > this:
> > 
> >   % zcat /usr/share/man/man1/git-add.1.gz | grep '\fB'
> >   you must use the \fBadd\fR command
> 
> As explained downthread, we don't know in the manual pages what font
> styling we're in.  troff has font-change commands, not nesting begin-end
> pairs, for italics and bold.  If the linkgit macro appears in the middle
> of a passage in italics, by not using \fP, we'll force the rest of the
> text which is to be italicized into roman.
> 
> The toolchain, whether Asciidoctor or the XSLT stylesheets, _does_ have
> this context and therefore can explicitly move between bold and roman,
> but our extensions do not.

Indeed but it's rare (there's probably zero instances), and it increases
the delta. Yes, it's more correct, but it trades a hypothetical benefit
for a real disadvantage.

Either way I see no point in arguing about this. If you feel strongly
about this I can include it in my version.

> > > Additionally, we don't want to use XML-style escapes for the litdd and
> > > plus macros, so let's only use the XML-style escapes in HTML and XML and
> > > use something different for our man pages.
> > 
> > That's a fourth change now, and one that complicates the Makefile even
> > more, when I've been trying to simplify it.
> 
> I'm sorry that this complicates work you'd like to do, but
> unfortunately, the other option is broken rendering.

Clean and maintainable code is a benefit to the project, not just me.
And the Makefile is code too.

Forget about me, a clean and simple Makefile is better than a cluttered
and complex Makefile. That is the point.

Yes, the rendering is "broken" without the change (that's loaded
language, but OK), we want to know precisely how, and how it got fixed.
We don't wan to sneak in all the fixes in the world in the first patch.

Moreover, there's no dicotomy here; we can fix the "broken" state in
other ways that don't complicate the Makefile, as I did in my patch
series [1].

But in fact, I have an even simpler version now:

 Asciidoctor::Extensions.register do
+  # Override attributes for man pages.
+  # https://github.com/asciidoctor/asciidoctor/issues/4059
+  if document.backend == 'manpage'
+    document.attributes.merge!({ 'litdd' => '\--', 'plus' => '+' })
+  end
+
   inline_macro Git::Documentation::LinkGitProcessor, :linkgit
   postprocessor Git::Documentation::DocumentPostProcessor
 end

> > > If users are using a more modern toolchain or don't care
> > > about the rendering issues, they can enable the option.
> > 
> > What rendering issues?
> 
> They were mentioned upthread.

No. Pepole mentioned issues they *think* existed, nobody pointed out an
actual reproducible issue.

As you experienced with the \e setback, the issue could have been with
their build, or it could be present in v1.5.7, but not v1.5.8. It's hard
to know if nobody spells out what the issue is.

So... What rendering issues?

> > Also, the many should not suffer because of the few.
> > 
> > If a few people doing USE_ASCIIDOCTOR=YesPlease have issues (because of
> > ancient packages in their distribution, and their reluctance to type
> > `gem install`), then *they* can disable USE_ASCIIDOCTOR_MANPAGE (or just
> > disable USE_ASCIIDOCTOR altogether). Most people doing
> > USE_ASCIIDOCTOR=YesPlease should not suffer because of a
> > minority.
> 
> I don't believe we're going to agree on this.  I believe we should
> choose defaults that work with the most popular Linux distributions, and
> you don't.

Untrue.

`make USE_ASCIIDOCTOR= doc` works perfectly fine on Debian stable with
my patches, and that's the default, nobody is changing that.

And so does this:

  gem install asciidoctor && make USE_ASCIIDOCTOR=1 doc

But in fact, so does `make USE_ASCIIDOCTOR=1 doc`, because asciidoctor
1.5.8 works just fine. I just did a doc-diff with v1.5.8, and I
checked everything without finding any serious issue (not present in
2.0.15).

The only issue is that \\ was not handled correctly, but I now have a
workaround for that:

  if document.basebackend?('manpage') and Asciidoctor::VERSION < '2.0.11'
    postprocessor do
      process do |document, output|
        output.gsub("\\(rs\\\\", "\\(rs\\(rs\\")
      end
    end
  end

> I think your approach is unnecessarily hostile to ordinary
> users and developers and understates the value that people derive from
> distributions.

Please tell me exactly how my patches are hostile to "ordinary
developers" (who are not ordinary at all).

 1. Will they be able to build the documentation with default flags?
    Yes
 2. Will they be able to build with USE_ASCIIDOCTOR=1?
    Yes
 3. Will they be able to see a reasonable output?
    Yes
 4. Will they be forced to install a gem?
    No

So where exactly is the hostility?

> > > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > 
> > I most definitely do not sign off this.
> 
> This sign-off is not an approval of the patch.

Yes it is. According to the Developer Certificate of Origin [2] I must:

  agree that a record of the contribution (including my sign-off) is
  maintained indefinitely

Which I don't.

Feel free to disregard my lack of agreement, but I'm stating it for the
record.

> I do want to be clear that I'm aware you don't approve of this patch and
> that's why I submitted a counterproposal: because I don't approve of
> your patch

That happens.

> and you seem unwilling to make changes to it.

Just because I disagree with your changes doesn't mean I'm unwilling to
make changes to my patch, especially since I already agreed on making
changes to my patch.

> I would love nothing more than to remove your name from it entirely,
> but unfortunately, that's not possible with the DCO.

It's not possible *if* 1) you use my code, and 2) you made changes I'm
opposed to. But you are already in violation of the DCO anyway by
disregarding my agreement, which is mandatory.

If you don't want to be in violation of the DCO you could try to fix
either 1) or 2).

Cheers.

[1] https://lore.kernel.org/git/20210514121435.504423-8-felipe.contreras@gmail.com/T/#u
[2] https://developercertificate.org/
diff mbox series

Patch

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 2aae4c9cbb..891181c0f3 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -192,10 +192,16 @@  ASCIIDOC_HTML = xhtml5
 ASCIIDOC_DOCBOOK = docbook5
 ASCIIDOC_EXTRA += -acompat-mode -atabsize=8
 ASCIIDOC_EXTRA += -I. -rasciidoctor-extensions
-ASCIIDOC_EXTRA += -alitdd='&\#x2d;&\#x2d;'
+TXT_TO_HTML += -alitdd='&\#x2d;&\#x2d;'
+TXT_TO_XML  += -alitdd='&\#x2d;&\#x2d;'
 DBLATEX_COMMON =
 XMLTO_EXTRA += --skip-validation
 XMLTO_EXTRA += -x manpage.xsl
+ifdef USE_ASCIIDOCTOR_MANPAGE
+TXT_TO_MAN = $(ASCIIDOC_COMMON) -b manpage
+TXT_TO_MAN += -aplus='+'
+TXT_TO_MAN += -alitdd='\--'
+endif
 endif
 
 SHELL_PATH ?= $(SHELL)
@@ -334,7 +340,7 @@  mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
 		show_tool_names can_merge "* " || :' >mergetools-merge.txt && \
 	date >$@
 
-TRACK_ASCIIDOCFLAGS = $(subst ','\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK))
+TRACK_ASCIIDOCFLAGS = $(subst ','\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK):$(USE_ASCIIDOCTOR_MANPAGE))
 
 GIT-ASCIIDOCFLAGS: FORCE
 	@FLAGS='$(TRACK_ASCIIDOCFLAGS)'; \
@@ -367,9 +373,16 @@  $(OBSOLETE_HTML): %.html : %.txto asciidoc.conf asciidoctor-extensions.rb GIT-AS
 manpage-base-url.xsl: manpage-base-url.xsl.in
 	$(QUIET_GEN)sed "s|@@MAN_BASE_URL@@|$(MAN_BASE_URL)|" $< > $@
 
+ifdef TXT_TO_MAN
+%.1 %.5 %.7 : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
+	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
+	$(TXT_TO_MAN) -o $@+ $< && \
+	mv $@+ $@
+else
 %.1 %.5 %.7 : %.xml manpage-base-url.xsl $(wildcard manpage*.xsl)
 	$(QUIET_XMLTO)$(RM) $@ && \
 	$(XMLTO) -m $(MANPAGE_XSL) $(XMLTO_EXTRA) man $<
+endif
 
 %.xml : %.txt asciidoc.conf asciidoctor-extensions.rb GIT-ASCIIDOCFLAGS
 	$(QUIET_ASCIIDOC)$(RM) $@+ $@ && \
diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
index d906a00803..620b3d7a88 100644
--- a/Documentation/asciidoctor-extensions.rb
+++ b/Documentation/asciidoctor-extensions.rb
@@ -15,6 +15,8 @@  module Git
           "#{target}(#{attrs[1]})</ulink>"
         elsif parent.document.basebackend? 'html'
           %(<a href="#{prefix}#{target}.html">#{target}(#{attrs[1]})</a>)
+        elsif parent.document.basebackend? 'manpage'
+          %(\e\\fB#{target}\e\\fP\e\\fR(#{attrs[1]})\e\\fP)
         elsif parent.document.basebackend? 'docbook'
           "<citerefentry>\n" \
             "<refentrytitle>#{target}</refentrytitle>" \
diff --git a/Makefile b/Makefile
index 93664d6714..e499152ba2 100644
--- a/Makefile
+++ b/Makefile
@@ -285,6 +285,10 @@  all::
 # Define USE_ASCIIDOCTOR to use Asciidoctor instead of AsciiDoc to build the
 # documentation.
 #
+# Define USE_ASCIIDOCTOR_MANPAGE to use Asciidoctor's manual page backend
+# instead of building manual pages from DocBook (using xmlto).  Has no effect
+# unless USE_ASCIIDOCTOR is set.
+#
 # Define ASCIIDOCTOR_EXTENSIONS_LAB to point to the location of the Asciidoctor
 # Extensions Lab if you have it available.
 #