Re: [PATCH v2 1/2] config: allow giving separate author and committer idents

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Jan 28 2019, William Hubbs wrote:

> On Fri, Jan 25, 2019 at 11:58:10PM +0100, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Fri, Jan 25 2019, William Hubbs wrote:
>>
>> > diff --git a/Documentation/config/user.txt b/Documentation/config/user.txt
>> > index b5b2ba1199..18e1ec3c1b 100644
>> > --- a/Documentation/config/user.txt
>> > +++ b/Documentation/config/user.txt
>> > @@ -1,12 +1,39 @@
>> > +author.email::
>> > +	The email address used for the author of newly
>> > +	created commits.  Defaults to the value of the
>> > +	`GIT_AUTHOR_EMAIL` environment variable, or if
>> > +	the environment variable is not set, the `user.email`
>> > +	configuration variable.
>> > +
>> > +author.name::
>> > +	The full name used for the author of newly created commits.
>> > +	Defaults to the value of the `GIT_AUTHOR_NAME` environment variable, or
>> > +	if the environment variable is not set,
>> > +	the `user.email` configuration variable.
>> > +
>> > +committer.email::
>> > +	The email address used for the committer of newly created commits.
>> > +	Defaults to the value of the `GIT_COMMITTER_EMAIL` environment
>> > +	variable, or if the environment variable is not set, the `user.email`
>> > +	configuration variable.
>> > +
>> > +committer.name::
>> > +	The full name used for the committer of newly created commits.
>> > +	Defaults to the value of the `GIT_COMMITTER_NAME` environment
>> > +	variable, or if the environment variable is not set, the `user.name`
>> > +	configuration variable.
>> > +
>> >  user.email::
>> >  	Your email address to be recorded in any newly created commits.
>> >  	Can be overridden by the `GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_EMAIL`, and
>> > -	`EMAIL` environment variables.  See linkgit:git-commit-tree[1].
>> > +	`EMAIL` environment variables or the `author.email` or
>> > +	`committer.email` settings discussed above. See linkgit:git-commit-tree[1].
>> >
>> >  user.name::
>> >  	Your full name to be recorded in any newly created commits.
>> >  	Can be overridden by the `GIT_AUTHOR_NAME` and `GIT_COMMITTER_NAME`
>> > -	environment variables.  See linkgit:git-commit-tree[1].
>> > +	environment variables or the `author.name` or `committer.name`
>> > +	settings discussed above. See linkgit:git-commit-tree[1].
>>
>> Looks correct, although I wonder if we're at the point where it would be
>> better to present this info as a table.
>
> Maybe, but can we have someone do that in a separate patch? I ask
> because the documentation is not in a markup language and that would
> make setting up a table difficult for me at best with my screen reader.

I'm happy to help if you'd like. I had a thinko with "table", and I
think our asciidoc dialect doesn't support it (maybe I'm wrong), but
thinking about it again we could just describe these variables all in
the same documentation. As in this hunk (which you could squash in):

BEGIN QUOTE
diff --git a/Documentation/config/user.txt b/Documentation/config/user.txt
index 18e1ec3c1b..ad3c43cf47 100644
--- a/Documentation/config/user.txt
+++ b/Documentation/config/user.txt
@@ -1,39 +1,20 @@
-author.email::
-	The email address used for the author of newly
-	created commits.  Defaults to the value of the
-	`GIT_AUTHOR_EMAIL` environment variable, or if
-	the environment variable is not set, the `user.email`
-	configuration variable.
-
+user.name::
+user.email::
 author.name::
-	The full name used for the author of newly created commits.
-	Defaults to the value of the `GIT_AUTHOR_NAME` environment variable, or
-	if the environment variable is not set,
-	the `user.email` configuration variable.
-
-committer.email::
-	The email address used for the committer of newly created commits.
-	Defaults to the value of the `GIT_COMMITTER_EMAIL` environment
-	variable, or if the environment variable is not set, the `user.email`
-	configuration variable.
-
+author.email::
 committer.name::
-	The full name used for the committer of newly created commits.
-	Defaults to the value of the `GIT_COMMITTER_NAME` environment
-	variable, or if the environment variable is not set, the `user.name`
-	configuration variable.
-
-user.email::
-	Your email address to be recorded in any newly created commits.
-	Can be overridden by the `GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_EMAIL`, and
-	`EMAIL` environment variables or the `author.email` or
-	`committer.email` settings discussed above. See linkgit:git-commit-tree[1].
-
-user.name::
-	Your full name to be recorded in any newly created commits.
-	Can be overridden by the `GIT_AUTHOR_NAME` and `GIT_COMMITTER_NAME`
-	environment variables or the `author.name` or `committer.name`
-	settings discussed above. See linkgit:git-commit-tree[1].
+committer.email::
+	The `user.name` and `user.email` variables determine what ends
+	up in the `author` and `committer` field of commit
+	objects. These config variables will be overridden by
+	`GIT_COMMITTER_NAME` and `GIT_COMMITTER_EMAIL`,
++
+Most users should have no reason to set the `author.*` and
+`committer.*` variables, but can do so to e.g. set different a
+different E-Mail for the `committer` field. Like the `user.name` and
+`user.email` variables, these can be overridden in the environment
+with `GIT_AUTHOR_NAME`, `GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_NAME` and
+`GIT_COMMITTER_EMAIL`.

 user.useConfigOnly::
 	Instruct Git to avoid trying to guess defaults for `user.email`
END QUOTE

Another thing I spotted while hacking that up is that the
git-commit-tree docs we're pointing to as the full explanation haven't
been updated. They still just talk about user.{name,email}.

And poking at this a bit more I see that something about this is
introducing new edge cases into our "you haven't set user.name or
user.email" logic. I.e. if I do:

    GIT_AUTHOR_NAME=hi GIT_AUTHOR_EMAIL=blah GIT_COMMITTER_NAME="hello" ./git-commit -a -m"hi"

I end up with an object like:

    author hi <blah> 1548705397 +0100
    committer hello <avar@nix.is> 1548705397 +0100

I.e. here I haven't supplied the committer E-Mail but it was inferred
(and a warning was printed to that effect). But if I do the same thing
with setting author.name etc. for all fields except committer.email I'll
get an empty ("<>") field for the committer E-Mail, even though it
printed "Your name and email address were configured automatically based
on your username and hostname".


>> > diff --git a/builtin/am.c b/builtin/am.c
>> > index 95370313b6..53fdd22c45 100644
>> > --- a/builtin/am.c
>> > +++ b/builtin/am.c
>> > @@ -1594,7 +1594,7 @@ static void do_commit(const struct am_state *state)
>> >  	}
>> >
>> >  	author = fmt_ident(state->author_name, state->author_email,
>> > -			state->ignore_date ? NULL : state->author_date,
>> > +			WANT_AUTHOR_IDENT, state->ignore_date ? NULL : state->author_date,
>>
>> This & a few other things in this series take the code beyond 79
>> characters.
>
> This doesn't look like it is beyond 79 characters to me, but that may be
> because I use a tab stop width of 4.
>
> Can you reply again and flag the lines that are longer than 79
> characters?

I see Junio replied to this already.

Adjusting for limited time, I'm happy to help out with this series,
particularly if you have visual (screen reader) issues that make some of
this prohibitive for you. Just say what you need.