Git Mailing List Archive mirror
 help / color / mirror / Atom feed
* [PATCH] doc: manpage: remove maximum title length
@ 2023-05-03  5:29 Felipe Contreras
  2023-05-03 16:43 ` Jeff King
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Felipe Contreras @ 2023-05-03  5:29 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Felipe Contreras

DocBook Stylesheets limit the size of the manpage titles for some
reason.

Even some of the longest git commands have no trouble fitting in 80
character terminals, so it's not clear why we would want to limit titles
to 20 characters, especially when modern terminals are much bigger.

For example:

--- a/git-credential-cache--daemon.1
+++ b/git-credential-cache--daemon.1
@@ -1,4 +1,4 @@
-GIT-CREDENTIAL-CAC(1)             Git Manual             GIT-CREDENTIAL-CAC(1)
+GIT-CREDENTIAL-CACHE--DAEMON(1)   Git Manual   GIT-CREDENTIAL-CACHE--DAEMON(1)

 NAME
        git-credential-cache--daemon - Temporarily store user credentials in
@@ -24,4 +24,4 @@ DESCRIPTION
 GIT
        Part of the git(1) suite

-Git omitted                       2023-05-02             GIT-CREDENTIAL-CAC(1)
+Git omitted                       2023-05-02   GIT-CREDENTIAL-CACHE--DAEMON(1)

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/manpage-normal.xsl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/manpage-normal.xsl b/Documentation/manpage-normal.xsl
index a9c7ec69f4..e7aa5df2fc 100644
--- a/Documentation/manpage-normal.xsl
+++ b/Documentation/manpage-normal.xsl
@@ -8,6 +8,9 @@
 <xsl:param name="man.output.quietly" select="1"/>
 <xsl:param name="refentry.meta.get.quietly" select="1"/>
 
+<!-- unset maximum length of title -->
+<xsl:param name="man.th.title.max.length"/>
+
 <!-- convert asciidoc callouts to man page format -->
 <xsl:template match="co">
 	<xsl:value-of select="concat('\fB(',substring-after(@id,'-'),')\fR')"/>
-- 
2.40.0+fc1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] doc: manpage: remove maximum title length
  2023-05-03  5:29 [PATCH] doc: manpage: remove maximum title length Felipe Contreras
@ 2023-05-03 16:43 ` Jeff King
  2023-05-03 17:09   ` Felipe Contreras
  2023-05-03 17:16 ` [PATCH v2] " Felipe Contreras
  2023-05-03 17:43 ` [PATCH] " Junio C Hamano
  2 siblings, 1 reply; 8+ messages in thread
From: Jeff King @ 2023-05-03 16:43 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git

On Tue, May 02, 2023 at 11:29:26PM -0600, Felipe Contreras wrote:

> DocBook Stylesheets limit the size of the manpage titles for some
> reason.
> 
> Even some of the longest git commands have no trouble fitting in 80
> character terminals, so it's not clear why we would want to limit titles
> to 20 characters, especially when modern terminals are much bigger.

Makes sense.

Since the manpage header shows the name twice, along with "Git Manual",
the practical limit for an 80-column terminal is somewhere around 35
characters. If it's not hard to do, it might be worth setting the value
there, but I agree that we're unlikely to exceed that anyway, so it's
probably not a big deal either way.

I could also see an argument that the truncation is worse than any
wrapping or other ugliness that the user might see on a smaller
terminal, which implies that "no limit" as you have here is the best
option.

> For example:
> 
> --- a/git-credential-cache--daemon.1
> +++ b/git-credential-cache--daemon.1
> @@ -1,4 +1,4 @@
> -GIT-CREDENTIAL-CAC(1)             Git Manual             GIT-CREDENTIAL-CAC(1)
> +GIT-CREDENTIAL-CACHE--DAEMON(1)   Git Manual   GIT-CREDENTIAL-CACHE--DAEMON(1)
> 
>  NAME
>         git-credential-cache--daemon - Temporarily store user credentials in
> @@ -24,4 +24,4 @@ DESCRIPTION
>  GIT
>         Part of the git(1) suite
> 
> -Git omitted                       2023-05-02             GIT-CREDENTIAL-CAC(1)
> +Git omitted                       2023-05-02   GIT-CREDENTIAL-CACHE--DAEMON(1)

Your patch can't be applied by "git am" because of this diff in the
commit message (it thinks the commit message stops at the first diff,
even if it is before a "---" marker). The usual practice is to indent
the included diff.

-Peff

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] doc: manpage: remove maximum title length
  2023-05-03 16:43 ` Jeff King
@ 2023-05-03 17:09   ` Felipe Contreras
  0 siblings, 0 replies; 8+ messages in thread
From: Felipe Contreras @ 2023-05-03 17:09 UTC (permalink / raw)
  To: Jeff King, Felipe Contreras; +Cc: git

Jeff King wrote:
> On Tue, May 02, 2023 at 11:29:26PM -0600, Felipe Contreras wrote:
> 
> > DocBook Stylesheets limit the size of the manpage titles for some
> > reason.
> > 
> > Even some of the longest git commands have no trouble fitting in 80
> > character terminals, so it's not clear why we would want to limit titles
> > to 20 characters, especially when modern terminals are much bigger.
> 
> Makes sense.
> 
> Since the manpage header shows the name twice, along with "Git Manual",
> the practical limit for an 80-column terminal is somewhere around 35
> characters. If it's not hard to do, it might be worth setting the value
> there,

It's not hard to do, but that would create a discrepancy with asciidoctor
manpage backend, which doesn't do that.

> I could also see an argument that the truncation is worse than any
> wrapping or other ugliness that the user might see on a smaller
> terminal, which implies that "no limit" as you have here is the best
> option.

I think it's best to remove the limit so there are no surprises and the output
is consistent among tools.

> > For example:
> > 
> > --- a/git-credential-cache--daemon.1
> > +++ b/git-credential-cache--daemon.1
> > @@ -1,4 +1,4 @@
> > -GIT-CREDENTIAL-CAC(1)             Git Manual             GIT-CREDENTIAL-CAC(1)
> > +GIT-CREDENTIAL-CACHE--DAEMON(1)   Git Manual   GIT-CREDENTIAL-CACHE--DAEMON(1)
> > 
> >  NAME
> >         git-credential-cache--daemon - Temporarily store user credentials in
> > @@ -24,4 +24,4 @@ DESCRIPTION
> >  GIT
> >         Part of the git(1) suite
> > 
> > -Git omitted                       2023-05-02             GIT-CREDENTIAL-CAC(1)
> > +Git omitted                       2023-05-02   GIT-CREDENTIAL-CACHE--DAEMON(1)
> 
> Your patch can't be applied by "git am" because of this diff in the
> commit message (it thinks the commit message stops at the first diff,
> even if it is before a "---" marker). The usual practice is to indent
> the included diff.

All right, I'll send an update.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] doc: manpage: remove maximum title length
  2023-05-03  5:29 [PATCH] doc: manpage: remove maximum title length Felipe Contreras
  2023-05-03 16:43 ` Jeff King
@ 2023-05-03 17:16 ` Felipe Contreras
  2023-05-03 17:58   ` Junio C Hamano
  2023-05-03 17:43 ` [PATCH] " Junio C Hamano
  2 siblings, 1 reply; 8+ messages in thread
From: Felipe Contreras @ 2023-05-03 17:16 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Felipe Contreras

DocBook Stylesheets limit the size of the manpage titles for some
reason.

Even some of the longest git commands have no trouble fitting in 80
character terminals, so it's not clear why we would want to limit titles
to 20 characters, especially when modern terminals are much bigger.

For example:

  --- a/git-credential-cache--daemon.1
  +++ b/git-credential-cache--daemon.1
  @@ -1,4 +1,4 @@
  -GIT-CREDENTIAL-CAC(1)             Git Manual             GIT-CREDENTIAL-CAC(1)
  +GIT-CREDENTIAL-CACHE--DAEMON(1)   Git Manual   GIT-CREDENTIAL-CACHE--DAEMON(1)

   NAME
          git-credential-cache--daemon - Temporarily store user credentials in
  @@ -24,4 +24,4 @@ DESCRIPTION
   GIT
          Part of the git(1) suite

  -Git omitted                       2023-05-02             GIT-CREDENTIAL-CAC(1)
  +Git omitted                       2023-05-02   GIT-CREDENTIAL-CACHE--DAEMON(1)

Moreover, asciidoctor manpage backend doesn't limit the title length, so
we probably want to do the same for docbook backends for consistency.

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

Since v1 the example diff in the commit message has been indented and an
extra paragraph explaining the situation with asciidoctor manpage
backend was added.

Range-diff against v1:
1:  b356b40db1 ! 1:  ea17191bd5 doc: manpage: remove maximum title length
    @@ Commit message
     
         For example:
     
    -    --- a/git-credential-cache--daemon.1
    -    +++ b/git-credential-cache--daemon.1
    -    @@ -1,4 +1,4 @@
    -    -GIT-CREDENTIAL-CAC(1)             Git Manual             GIT-CREDENTIAL-CAC(1)
    -    +GIT-CREDENTIAL-CACHE--DAEMON(1)   Git Manual   GIT-CREDENTIAL-CACHE--DAEMON(1)
    +      --- a/git-credential-cache--daemon.1
    +      +++ b/git-credential-cache--daemon.1
    +      @@ -1,4 +1,4 @@
    +      -GIT-CREDENTIAL-CAC(1)             Git Manual             GIT-CREDENTIAL-CAC(1)
    +      +GIT-CREDENTIAL-CACHE--DAEMON(1)   Git Manual   GIT-CREDENTIAL-CACHE--DAEMON(1)
     
    -     NAME
    -            git-credential-cache--daemon - Temporarily store user credentials in
    -    @@ -24,4 +24,4 @@ DESCRIPTION
    -     GIT
    -            Part of the git(1) suite
    +       NAME
    +              git-credential-cache--daemon - Temporarily store user credentials in
    +      @@ -24,4 +24,4 @@ DESCRIPTION
    +       GIT
    +              Part of the git(1) suite
     
    -    -Git omitted                       2023-05-02             GIT-CREDENTIAL-CAC(1)
    -    +Git omitted                       2023-05-02   GIT-CREDENTIAL-CACHE--DAEMON(1)
    +      -Git omitted                       2023-05-02             GIT-CREDENTIAL-CAC(1)
    +      +Git omitted                       2023-05-02   GIT-CREDENTIAL-CACHE--DAEMON(1)
    +
    +    Moreover, asciidoctor manpage backend doesn't limit the title length, so
    +    we probably want to do the same for docbook backends for consistency.
     
         Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
     

 Documentation/manpage-normal.xsl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/manpage-normal.xsl b/Documentation/manpage-normal.xsl
index a9c7ec69f4..e7aa5df2fc 100644
--- a/Documentation/manpage-normal.xsl
+++ b/Documentation/manpage-normal.xsl
@@ -8,6 +8,9 @@
 <xsl:param name="man.output.quietly" select="1"/>
 <xsl:param name="refentry.meta.get.quietly" select="1"/>
 
+<!-- unset maximum length of title -->
+<xsl:param name="man.th.title.max.length"/>
+
 <!-- convert asciidoc callouts to man page format -->
 <xsl:template match="co">
 	<xsl:value-of select="concat('\fB(',substring-after(@id,'-'),')\fR')"/>
-- 
2.40.0+fc1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] doc: manpage: remove maximum title length
  2023-05-03  5:29 [PATCH] doc: manpage: remove maximum title length Felipe Contreras
  2023-05-03 16:43 ` Jeff King
  2023-05-03 17:16 ` [PATCH v2] " Felipe Contreras
@ 2023-05-03 17:43 ` Junio C Hamano
  2023-05-03 17:49   ` Junio C Hamano
  2023-05-08  0:19   ` Felipe Contreras
  2 siblings, 2 replies; 8+ messages in thread
From: Junio C Hamano @ 2023-05-03 17:43 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Jeff King

Felipe Contreras <felipe.contreras@gmail.com> writes:

> DocBook Stylesheets limit the size of the manpage titles for some
> reason.
>
> Even some of the longest git commands have no trouble fitting in 80
> character terminals, so it's not clear why we would want to limit titles
> to 20 characters, especially when modern terminals are much bigger.

I agree with the general thrust, but I do not think we are the ones
who limit it to 20.  It is a value that is "reasonable but somewhat
arbitrary", decided by somebody, and may even vary across installed
versions of DocBook XSL Stylesheets and their customizations, isn't
it (in other words, for some readers of "git log", it may not even
be 20, if their distro tweaked the value to suit their needs)?

Perhaps rephrase it ...

    DocBook Stylesheets limit the size of the manpage titles to
    avoid it (often shown twice, from both ends of the page)
    overlapping with other elements on the same line, such as the
    section name (for us, "Git Manual").  They say it is set to a
    "reasonable but somewhat arbitrary" value by default, and
    encourage "experiment with changing the value in order to
    achieve the correct aesthetic results, where they document the
    man.th.title.max.length parameter [*].

    The longest title we need to show for the Git manual pages
    currently is "git-credential-cache--daemon(1)" that is 30
    characters long, but I've seen on my box with docbook-xsl
    1.79.2+dfsg-2 installed that the "reasonable" default was set to
    20, which would cause the title shown like so:

       GIT-CREDENTIAL-CAC(1)             Git Manual             GIT-CREDENTIAL-CAC(1)

    We could raise the limit to, say, 32 as a conservative choice
    and can get this line show the full title:

       GIT-CREDENTIAL-CACHE--DAEMON(1)   Git Manual   GIT-CREDENTIAL-CACHE--DAEMON(1)

    but because even the longest one we currently have would fit on
    an 80-column terminal, let's make it unlimited for now.

    [Reference]
    * https://cdn.docbook.org/release/xsl/snapshot/doc/manpages/man.th.title.max.length.html

... or something along that line.

I did NOT verify the claim that even the longest will fit in
80-column limit, that credential-cache--daemon is the longest one,
or that the box the problem was observed was using which version of
the stylesheet.  The above example illustrates the level of detail
needed for a proper log message, but it may contain factual errors
that need to be updated when the patch gets rerolled.

FWIW, my primary motivation behind suggesting update of the above
log message was to make sure that we document that we made a
conscious decision to make it unlimited, instead of choosing another
arbitrary limit (which we can do when we actually need to).

Thanks.

> ---
>  Documentation/manpage-normal.xsl | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/manpage-normal.xsl b/Documentation/manpage-normal.xsl
> index a9c7ec69f4..e7aa5df2fc 100644
> --- a/Documentation/manpage-normal.xsl
> +++ b/Documentation/manpage-normal.xsl
> @@ -8,6 +8,9 @@
>  <xsl:param name="man.output.quietly" select="1"/>
>  <xsl:param name="refentry.meta.get.quietly" select="1"/>
>  
> +<!-- unset maximum length of title -->
> +<xsl:param name="man.th.title.max.length"/>
> +
>  <!-- convert asciidoc callouts to man page format -->
>  <xsl:template match="co">
>  	<xsl:value-of select="concat('\fB(',substring-after(@id,'-'),')\fR')"/>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] doc: manpage: remove maximum title length
  2023-05-03 17:43 ` [PATCH] " Junio C Hamano
@ 2023-05-03 17:49   ` Junio C Hamano
  2023-05-08  0:19   ` Felipe Contreras
  1 sibling, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2023-05-03 17:49 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Jeff King

Junio C Hamano <gitster@pobox.com> writes:

> FWIW, my primary motivation behind suggesting update of the above
> log message was to make sure that we document that we made a
> conscious decision to make it unlimited, instead of choosing another
> arbitrary limit (which we can do when we actually need to).

Elsewhere I think I saw "asciidoctor does not limit" mentioned.  If
that is the case, then it is a very good thing to throw in as a
justification why we chose to make it unlimited instead of raising
the default limit.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] doc: manpage: remove maximum title length
  2023-05-03 17:16 ` [PATCH v2] " Felipe Contreras
@ 2023-05-03 17:58   ` Junio C Hamano
  0 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2023-05-03 17:58 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Jeff King

Felipe Contreras <felipe.contreras@gmail.com> writes:

> DocBook Stylesheets limit the size of the manpage titles for some
> reason.
> ...
> Moreover, asciidoctor manpage backend doesn't limit the title length, so
> we probably want to do the same for docbook backends for consistency.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---

Looking good.  It is especially good that we say why we chose to
make it unlimited instead of raising it to some arbitrary value.

The claim that the longest one would fit on a line is not still
substantiated (we could say "git-X manual page" needs the most
columns and people can check for themselves), but I'll let it pass.

Will queue.  Thanks.

> diff --git a/Documentation/manpage-normal.xsl b/Documentation/manpage-normal.xsl
> index a9c7ec69f4..e7aa5df2fc 100644
> --- a/Documentation/manpage-normal.xsl
> +++ b/Documentation/manpage-normal.xsl
> @@ -8,6 +8,9 @@
>  <xsl:param name="man.output.quietly" select="1"/>
>  <xsl:param name="refentry.meta.get.quietly" select="1"/>
>  
> +<!-- unset maximum length of title -->
> +<xsl:param name="man.th.title.max.length"/>
> +
>  <!-- convert asciidoc callouts to man page format -->
>  <xsl:template match="co">
>  	<xsl:value-of select="concat('\fB(',substring-after(@id,'-'),')\fR')"/>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] doc: manpage: remove maximum title length
  2023-05-03 17:43 ` [PATCH] " Junio C Hamano
  2023-05-03 17:49   ` Junio C Hamano
@ 2023-05-08  0:19   ` Felipe Contreras
  1 sibling, 0 replies; 8+ messages in thread
From: Felipe Contreras @ 2023-05-08  0:19 UTC (permalink / raw)
  To: Junio C Hamano, Felipe Contreras; +Cc: git, Jeff King

Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
> 
> > DocBook Stylesheets limit the size of the manpage titles for some
> > reason.
> >
> > Even some of the longest git commands have no trouble fitting in 80
> > character terminals, so it's not clear why we would want to limit titles
> > to 20 characters, especially when modern terminals are much bigger.
> 
> I agree with the general thrust, but I do not think we are the ones
> who limit it to 20.

We are not explicitely telling docbook-xsl to limit the title to 20, no,
but we are limitting the tile to 20 by choosing to use docbook-xsl.

> It is a value that is "reasonable but somewhat arbitrary", decided by
> somebody, and may even vary across installed versions of DocBook XSL
> Stylesheets and their customizations, isn't it (in other words, for some
> readers of "git log", it may not even be 20,

No. Just because docbook-xsl says X doesn't mean X is true.

> if their distro tweaked the value to suit their needs)?
> 
> Perhaps rephrase it ...
> 
>     DocBook Stylesheets limit the size of the manpage titles to
>     avoid it (often shown twice, from both ends of the page)
>     overlapping with other elements on the same line, such as the
>     section name (for us, "Git Manual").  They say it is set to a
>     "reasonable but somewhat arbitrary" value by default, and
>     encourage "experiment with changing the value in order to
>     achieve the correct aesthetic results, where they document the
>     man.th.title.max.length parameter [*].

But this isn't accurate. They don't limit the size of the manpage title, they
only do so *by default*.

>     The longest title we need to show for the Git manual pages
>     currently is "git-credential-cache--daemon(1)" that is 30
>     characters long, but I've seen on my box with docbook-xsl
>     1.79.2+dfsg-2 installed that the "reasonable" default was set to
>     20, which would cause the title shown like so:

This again is not accurate, because I'm 100% certain this is not just in
my box, as it's also the case in git-manpages.git:

https://git.kernel.org/pub/scm/git/git-manpages.git/tree/man1/git-credential-cache--daemon.1

>        GIT-CREDENTIAL-CAC(1)             Git Manual             GIT-CREDENTIAL-CAC(1)
> 
>     We could raise the limit to, say, 32 as a conservative choice
>     and can get this line show the full title:
> 
>        GIT-CREDENTIAL-CACHE--DAEMON(1)   Git Manual   GIT-CREDENTIAL-CACHE--DAEMON(1)
> 
>     but because even the longest one we currently have would fit on
>     an 80-column terminal, let's make it unlimited for now.

This is not why I think we should do it.

> ... or something along that line.

Except I don't agree with what was said on that line.

Let's keep in mind that docbook-xsl is not the only way to generate manpages,
so I don't think we should concern ourselves too much with what they do
or don't do, nor what they say or don't say.

> I did NOT verify the claim that even the longest will fit in
> 80-column limit, that credential-cache--daemon is the longest one,

Me neither, which is why I did not attempt to say that, but also: I
don't think it's particularly relevant which is the longest command
today, because tomorrow somebody might propose an even bigger one.

> or that the box the problem was observed was using which version of
> the stylesheet.

Again: doesn't matter.

> The above example illustrates the level of detail needed for a proper
> log message,

I don't think that level of detail is needed, or even desirable.

> FWIW, my primary motivation behind suggesting update of the above
> log message was to make sure that we document that we made a
> conscious decision to make it unlimited, instead of choosing another
> arbitrary limit (which we can do when we actually need to).

The conscious decision should be completely orthogonal to what
docbook-xsl developers considered a "reasonable default" some time ago
(probably decades ago).

The options for our conscious decision are:

 1. We don't limit the length of the title
 2. We limit the length of the title to some number

If we never made the conscious decision to limit the length of the title
to some arbitrary number, I think we should not do that (regardless of
docbook-xsl's default).

In other words: I don't think we should be burdened by the poor choices
of some other project regarding the default values of their weird
features.

Cheers.

-- 
Felipe Contreras

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-05-08  0:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-03  5:29 [PATCH] doc: manpage: remove maximum title length Felipe Contreras
2023-05-03 16:43 ` Jeff King
2023-05-03 17:09   ` Felipe Contreras
2023-05-03 17:16 ` [PATCH v2] " Felipe Contreras
2023-05-03 17:58   ` Junio C Hamano
2023-05-03 17:43 ` [PATCH] " Junio C Hamano
2023-05-03 17:49   ` Junio C Hamano
2023-05-08  0:19   ` Felipe Contreras

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).