Re: [PATCH] doc: simplify man version

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Apr 10, 2023, 23:32 UTC
Message-ID: <64349c7dc4f49_7452943c@chronos.notmuch>
In-Reply-To: <xmqqr0suf0wz.fsf@gitster.g>

Junio C Hamano wrote:

Show 18 quoted lines

> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > diff --git a/Documentation/Makefile b/Documentation/Makefile
> > index a6ba5bd460..4721b000c1 100644
> > --- a/Documentation/Makefile
> > +++ b/Documentation/Makefile
> > @@ -150,8 +150,7 @@ ASCIIDOC_HTML = xhtml11
> >  ASCIIDOC_DOCBOOK = docbook
> >  ASCIIDOC_CONF = -f asciidoc.conf
> >  ASCIIDOC_COMMON = $(ASCIIDOC) $(ASCIIDOC_EXTRA) $(ASCIIDOC_CONF) \
> > -		-amanversion=$(GIT_VERSION) \
> > -		-amanmanual='Git Manual' -amansource='Git'
> > +		-amanmanual='Git Manual' -amansource='Git $(GIT_VERSION)'
> >  ASCIIDOC_DEPS = asciidoc.conf GIT-ASCIIDOCFLAGS
> >  TXT_TO_HTML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_HTML)
> >  TXT_TO_XML = $(ASCIIDOC_COMMON) -b $(ASCIIDOC_DOCBOOK)
> 
> Is this a complete patch,

Yes it is complete.

> or will this leave us in an incomplete in-between place?

No.

Show 6 quoted lines

> We have some references to manversion in "git grep manversion
> Documentation/" in asciidoc.conf and asciidoctor-extensions.rb
> remaining after this ptach is applied, which presumably are no
> longer used.  I would imagine that these leftover references end up
> substituting them with something benign, like an empty string, in
> the output, but it somehow makes me feel dirty [*].

Passing an empty string has the same effect, because as it is explained in the commit message: DocBook Stylesheets simply join them *if* both are present (not empty).

> Other than that, I like the simplification of requiring only two
> pieces of information to convey the same information that we are
> attempting to (and to some backends, failing to) give with three
> pieces of information.

Yes.

Show 13 quoted lines

> [Footnote]
> 
> * If I am not guessing correctly how the result of applying this
>   patch works in the above "I would imagine ..." that led to my
>   possible misunderstanding of feeling "dirty", it would be a sign
>   that the proposed log message is not explaining sufficiently and
>   deserves an update.  Even just saying "... and when they join the
>   `source` and `version`, if `version` is left empty or unspecified,
>   the resulting document would not show any extra whitespace.  So it
>   is safe to do the joining ourselves and stuff the result in the
>   `source` field" or something would be sufficient, I would imagine,
>   in order to help the future readers of "git log" that there is no
>   need to "feel dirty" the same way I did.

I don't know know what could give this impression, given that a link to the documentation and the link to the source code was given:

  if we have a Name and/or Version, use either or both of those, in the
  form "Name Version" or just "Name" or just "Version"

https://github.com/docbook/xslt10-stylesheets/blob/master/xsl/common/refentry.xsl#L545

The code clearly tests for empty strings:

  test="not($Name = '') and not($Version = '')

And it's not clear to me what else it would be checking for.

asciidoc.py doesn't conditinally add this field: if manversion is not provided it just sets an empty field (if revnumber isn't provided either):

  <refmiscinfo class="version">{manversion={revnumber}}</refmiscinfo>

If this works for programs that don't set manversion, why wouldn't it work for us?

-- 
Felipe Contreras