Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Doc git commit #1845

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Doc git commit #1845

wants to merge 3 commits into from

Conversation

jnavila
Copy link

@jnavila jnavila commented Jan 2, 2025

This series continues the effort of rewriting the documentation with uniformization and better formatting of the man pages. This time, git-commit is processed, taking advantage of previous experiences.

cc: Patrick Steinhardt [email protected]

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.

Signed-off-by: Jean-Noël Avila <[email protected]>
Also prevent git-commit manpage to refer to itself in the config
description by using a variable.

Signed-off-by: Jean-Noël Avila <[email protected]>
@jnavila
Copy link
Author

jnavila commented Jan 3, 2025

/submit

Copy link

gitgitgadget bot commented Jan 3, 2025

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1845/jnavila/doc_git-commit-v1

To fetch this version to local tag pr-1845/jnavila/doc_git-commit-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1845/jnavila/doc_git-commit-v1

Copy link

gitgitgadget bot commented Jan 3, 2025

This patch series was integrated into seen via git@5686bb1.

@gitgitgadget gitgitgadget bot added the seen label Jan 3, 2025
@@ -7,8 +7,8 @@ git-commit - Record changes to the repository

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Patrick Steinhardt wrote (reply to this):

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?= <[email protected]>
> 
> 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 :)

> 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.

> @@ -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.

> 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?

Patrick

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Jean-Noël AVILA wrote (reply to this):

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?= <[email protected]>
> > 
> > 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
> 
> 



Copy link

gitgitgadget bot commented Jan 6, 2025

User Patrick Steinhardt <[email protected]> has been added to the cc: list.

Copy link

gitgitgadget bot commented Jan 6, 2025

This branch is now known as ja/doc-commit-markup-updates.

Copy link

gitgitgadget bot commented Jan 6, 2025

This patch series was integrated into seen via git@3ba590d.

Copy link

gitgitgadget bot commented Jan 6, 2025

There was a status update in the "Cooking" section about the branch ja/doc-commit-markup-updates on the Git mailing list:

Doc updates.
source: <[email protected]>

Copy link

gitgitgadget bot commented Jan 7, 2025

This patch series was integrated into seen via git@6ae1eb3.

Copy link

gitgitgadget bot commented Jan 8, 2025

This patch series was integrated into seen via git@ddd9fe1.

Copy link

gitgitgadget bot commented Jan 8, 2025

This patch series was integrated into seen via git@85cb907.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant