Re: [PATCH 1/3] doc: git-commit: apply new documentation guidelines

["Jean-Noël AVILA" <jn.avila@free.fr>]

Le lundi 6 janvier 2025, 08:38:19 CET Patrick Steinhardt a écrit :
> On Fri, Jan 03, 2025 at 01:47:24PM +0000, Jean-Noël Avila via GitGitGadget 
wrote:
> > From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
> > 
> > The documentation for git-commit has been updated to follow the new
> > documentation guidelines. The following changes have been applied to
> > the series of patches:
> > 
> > - switching the synopsis to a synopsis block which will automatically
> >   format placeholders in italics and keywords in monospace
> > - use _<placeholder>_ instead of <placeholder> in the description
> > - use `backticks for keywords and more complex option
> > descriptions`. The new rendering engine will apply synopsis rules to
> > these spans.
> > 
> > Additionally, some option descriptions have been turned into
> > imperative mood to make them more consistent with the rest of the
> > documentation.
> 
> Same comment here regarding the commit message as on the other two
> series. We should use imperative mood for it, as well :)

Thanks for reviewing.

Being consistent with myself is imperative ;-)

> 
> > diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> > index c822113c111..b08a398e31d 100644
> > --- a/Documentation/git-commit.txt
> > +++ b/Documentation/git-commit.txt
> > @@ -58,139 +58,139 @@ summary of what is included by any of the above for 
the next
> > --z::
> > ---null::
> > +`-z`::
> > +`--null`::
> >  	When showing `short` or `porcelain` status output, print the
> > -	filename verbatim and terminate the entries with NUL, instead of 
LF.
> > +	filename verbatim and terminate the entries with _NUL_, instead of 
_LF_.
> >  	If no format is given, implies the `--porcelain` output format.
> >  	Without the `-z` option, filenames with "unusual" characters are
> >  	quoted as explained for the configuration variable 
`core.quotePath`
> >  	(see linkgit:git-config[1]).
> >  
> > --F <file>::
> > ---file=<file>::
> > -	Take the commit message from the given file.  Use '-' to
> > +`-F <file>`::
> > +`--file=<file>`::
> > +	Take the commit message from _<file>_.  Use `-` to
> 
> I think it would make sense to move changes like this, where the actual
> wording changes, into a separate commit. That'd make it way easier to
> spot the non-mechanical changes from those that may require some
> discussion.

True. There are other parts where I introduced the place holder as an echo to 
the option description. The idea is to be less verbose and more contextual.

> 
> > @@ -257,19 +256,18 @@ default::
> >  The default can be changed by the `commit.cleanup` configuration
> >  variable (see linkgit:git-config[1]).
> >  
> > --e::
> > ---edit::
> > -	The message taken from file with `-F`, command line with
> > -	`-m`, and from commit object with `-C` are usually used as
> > -	the commit log message unmodified. This option lets you
> > -	further edit the message taken from these sources.
> > +`-e`::
> > +`--edit`::
> > +	Let the user further edit the message taken from  file
> 
> There's a double space here. I was also wondering whether this should
> say _<file>_ here to further clarify that this refers to the same
> placeholder as the placeholder in `-F`. Might be confusing though, I
> dunno.

This would sound a bit repetitive, but it is still better than making the 
reader refer to the entries for each option, in which case I would also add 
the placeholders when listing the options.

> 
> > diff --git a/builtin/commit.c b/builtin/commit.c
> > index ef5e622c077..a7315ed67cc 100644
> > --- a/builtin/commit.c
> > +++ b/builtin/commit.c
> > @@ -44,7 +44,7 @@
> >  #include "trailer.h"
> >  
> >  static const char * const builtin_commit_usage[] = {
> > -	N_("git commit [-a | --interactive | --patch] [-s] [-v] [-u<mode>] 
[--amend]\n"
> > +	N_("git commit [-a | --interactive | --patch] [-s] [-v] [-
u[<mode>]] [--amend]\n"
> >  	   "           [--dry-run] [(-c | -C | --squash) <commit> | --
fixup [(amend|reword):]<commit>]\n"
> >  	   "           [-F <file> | -m <msg>] [--reset-author] [--allow-
empty]\n"
> >  	   "           [--allow-empty-message] [--no-verify] [-e] [--
author=<author>]\n"
> 
> I guess this change is required to make t0450 happy?


Yes it is. To be honest, I was about to try to complete the synopsis for all 
commands, but finally let this task for another round, because I'm not even 
clear on what is alternative and what can be combined.

> 
> Patrick
> 
>