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

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: May 14, 2021, 19:02 UTC
Message-ID: <609ec960cc2f8_43127208a4@natae.notmuch>
In-Reply-To: <YJ24WIpnwE4LBOMt@camp.crustytoothpaste.net>

brian m. carlson wrote:

Show 12 quoted lines

> On 2021-05-12 at 04:41:41, Felipe Contreras wrote:
> > brian m. carlson wrote:
> > > since some common distros are > still on 1.5.
> > 
> > Are "some common distros" namely Debian stable *exclusively*?
> > 
> > If so, I would consider flipping the default the other way around,
> > espececially since it's only te default shipped by the Debian stable
> > packages (easily fixed by `gem install asciidoctor`).
> 
> CentOS 7 and Ubuntu 18.04 also ship 1.5.  CentOS 8 does not appear to
> ship Asciidoctor at all.

I does not matter what version of asciidoctor they *ship* with.... At all.

I happen to have accesss to an Ubuntu 18.04 machine...

  time gem install --user-install asciidoctor
  real	0m3.179s

It took me 3 seconds to get asciidoctor-2.0.15.

Ubuntu 18.04 ships with ruby 2.5.1p57 (2018-03-29 revision 63029). And that's all you need.

Show 9 quoted lines

> > > If users are using a more modern toolchain or don't care
> > > about the rendering issues, they can enable the option.
> > 
> > The other way around: if users are using an ancient distribution they
> > can disable the option.
> 
> Debian stable is not ancient.  It is less than two years old, which for
> a Linux distro or any operating system distribution, is not at all
> considered even reasonably old.

I guess your definition of what's "old" and mine are *very* different.

But the problem doesn't come from when the distribution was released, but when the packages *inside* the distribution were released.

> I am not going to propose or give my approval to a change that causes
> problems building Git with the distro packages on Debian stable or the
> latest Ubuntu LTS, in any way, shape, or form.  People should be able to
> use the distro packages if that's most convenient.

My proposed changes do not cause any problems building Git with any distro package on Debian stable or the latest Ubuntu LTS in any way, shape or form.

Show 9 quoted lines

> > > Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
> > > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > 
> > Commit-message-by: brian m. carlson <sandals@crustytoothpaste.net>
> > 
> > I certainly would not want to pretend to have written the text above.
> 
> I'll reroll with the author reset.

Thats is not what I just requested.

Show 20 quoted lines

> > > diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb
> > > index d906a00803..40fa87b121 100644
> > > --- a/Documentation/asciidoctor-extensions.rb
> > > +++ b/Documentation/asciidoctor-extensions.rb
> > > @@ -15,6 +15,8 @@ module Git
> > >            "#{target}(#{attrs[1]})</ulink>"
> > >          elsif parent.document.basebackend? 'html'
> > >            %(<a href="#{prefix}#{target}.html">#{target}(#{attrs[1]})</a>)
> > > +        elsif parent.document.basebackend? 'manpage'
> > > +          %(\\fB#{target}\\fP\\fR(#{attrs[1]})\\fP)
> > 
> > I still prefer my original version, especially since:
> > 
> >  1. I suspect most git developers are familiar with printf directives:
> >     %s.
> >  2. Where is that \\fP coming from? I don't see that with xmlto, nor the
> >     publicly genrated man pages[1].
> 
> That's coming from my knowledge of troff, having used it extensively
> over the years, and my general hesitance to affect global state.

Good. Send it as a *separate* patch.

Most people in the mailing list are trying to **minimize** the diff between asciidoc+docbook and asciidoctor, not piggyback improvements.

If you want to flex your troff knowledge do it as a separate patch, without my authorship, or s-o-b line.

Show 6 quoted lines

> >  3. Doesn't work on my machine without my original \e; I see
> >     "\fBgittutorial\fR(7)".
> 
> Works just fine on my system using Asciidoctor 2.0.12.  The \e would
> insert an additional escape character and shouldn't be needed unless
> we're in copy mode (which we should not be here).

I fail to see how that could work since asciidoctor replaces '\' with '\(rs'.

This is a simple test that reproduces the issue:

  ruby -r asciidoctor <<\EOF | groff -Tascii -man | less
  puts Asciidoctor.convert("This is a \\fBtest\\fR.", backend: :manpage)
  EOF

Those that work in your machine?

This happens both with v2.0.12 and the latest master (2.0.16.dev).

Cheers.

-- 
Felipe Contreras