[PATCH 0/3] Show author and/or committer in some cases

[PATCH 0/3] Show author and/or committer in some cases

[Santi Béjar]

Hi *,

  This patch series tries to make more evident the author and committer ident
used in a commit trying to be minimize when it is shown.

Santi

Santi Béjar (3):
  Preparation to call determine_author_info from prepare_to_commit
  commit: Show author if different from committer
  commit: Show the committer ident when is different from the parent

 builtin-commit.c  |  127 ++++++++++++++++++++++++++++++++++++-----------------
 t/t7502-commit.sh |   22 +++++++++
 2 files changed, 109 insertions(+), 40 deletions(-)

[PATCH 1/3] Preparation to call determine_author_info from prepare_to_commit

[Santi Béjar]

Reorder functions definitions such that determine_author_info is
defined before prepare_to_commit. No code changes.

Signed-off-by: Santi Béjar <sbejar@gmail.com>
---
 builtin-commit.c |   78 +++++++++++++++++++++++++++---------------------------
 1 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 256181a..a37d8c3 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -395,6 +395,45 @@ static int is_a_merge(const unsigned char *sha1)
 
 static const char sign_off_header[] = "Signed-off-by: ";
 
+static void determine_author_info(struct strbuf *sb)
+{
+	char *name, *email, *date;
+
+	name = getenv("GIT_AUTHOR_NAME");
+	email = getenv("GIT_AUTHOR_EMAIL");
+	date = getenv("GIT_AUTHOR_DATE");
+
+	if (use_message) {
+		const char *a, *lb, *rb, *eol;
+
+		a = strstr(use_message_buffer, "\nauthor ");
+		if (!a)
+			die("invalid commit: %s", use_message);
+
+		lb = strstr(a + 8, " <");
+		rb = strstr(a + 8, "> ");
+		eol = strchr(a + 8, '\n');
+		if (!lb || !rb || !eol)
+			die("invalid commit: %s", use_message);
+
+		name = xstrndup(a + 8, lb - (a + 8));
+		email = xstrndup(lb + 2, rb - (lb + 2));
+		date = xstrndup(rb + 2, eol - (rb + 2));
+	}
+
+	if (force_author) {
+		const char *lb = strstr(force_author, " <");
+		const char *rb = strchr(force_author, '>');
+
+		if (!lb || !rb)
+			die("malformed --author parameter");
+		name = xstrndup(force_author, lb - force_author);
+		email = xstrndup(lb + 2, rb - (lb + 2));
+	}
+
+	strbuf_addf(sb, "author %s\n", fmt_ident(name, email, date, IDENT_ERROR_ON_NO_NAME));
+}
+
 static int prepare_to_commit(const char *index_file, const char *prefix)
 {
 	struct stat statbuf;
@@ -622,45 +661,6 @@ static int message_is_empty(struct strbuf *sb, int start)
 	return 1;
 }
 
-static void determine_author_info(struct strbuf *sb)
-{
-	char *name, *email, *date;
-
-	name = getenv("GIT_AUTHOR_NAME");
-	email = getenv("GIT_AUTHOR_EMAIL");
-	date = getenv("GIT_AUTHOR_DATE");
-
-	if (use_message) {
-		const char *a, *lb, *rb, *eol;
-
-		a = strstr(use_message_buffer, "\nauthor ");
-		if (!a)
-			die("invalid commit: %s", use_message);
-
-		lb = strstr(a + 8, " <");
-		rb = strstr(a + 8, "> ");
-		eol = strchr(a + 8, '\n');
-		if (!lb || !rb || !eol)
-			die("invalid commit: %s", use_message);
-
-		name = xstrndup(a + 8, lb - (a + 8));
-		email = xstrndup(lb + 2, rb - (lb + 2));
-		date = xstrndup(rb + 2, eol - (rb + 2));
-	}
-
-	if (force_author) {
-		const char *lb = strstr(force_author, " <");
-		const char *rb = strchr(force_author, '>');
-
-		if (!lb || !rb)
-			die("malformed --author parameter");
-		name = xstrndup(force_author, lb - force_author);
-		email = xstrndup(lb + 2, rb - (lb + 2));
-	}
-
-	strbuf_addf(sb, "author %s\n", fmt_ident(name, email, date, IDENT_ERROR_ON_NO_NAME));
-}
-
 static int parse_and_validate_options(int argc, const char *argv[],
 				      const char * const usage[])
 {
-- 
1.5.5.1.148.g37726

[PATCH 2/3] commit: Show author if different from committer

[Santi Béjar]

That would help reassure anybody while committing other's changes.

Signed-off-by: Santi Béjar <sbejar@gmail.com>
---
 builtin-commit.c  |   23 ++++++++++++++++++++---
 t/t7502-commit.sh |   10 ++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index a37d8c3..c51c70f 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -47,6 +47,7 @@ static enum {
 
 static char *logfile, *force_author, *template_file;
 static char *edit_message, *use_message;
+static char *author_name, *author_email, *author_date;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, untracked_files, no_verify, allow_empty;
 /*
@@ -395,7 +396,7 @@ static int is_a_merge(const unsigned char *sha1)
 
 static const char sign_off_header[] = "Signed-off-by: ";
 
-static void determine_author_info(struct strbuf *sb)
+static void determine_author_info()
 {
 	char *name, *email, *date;
 
@@ -431,7 +432,9 @@ static void determine_author_info(struct strbuf *sb)
 		email = xstrndup(lb + 2, rb - (lb + 2));
 	}
 
-	strbuf_addf(sb, "author %s\n", fmt_ident(name, email, date, IDENT_ERROR_ON_NO_NAME));
+	author_name = name;
+	author_email = email;
+	author_date = date;
 }
 
 static int prepare_to_commit(const char *index_file, const char *prefix)
@@ -443,6 +446,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 	FILE *fp;
 	const char *hook_arg1 = NULL;
 	const char *hook_arg2 = NULL;
+	const char *author_ident;
+	const char *committer_ident;
 
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		return 0;
@@ -522,6 +527,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 
 	strbuf_release(&sb);
 
+	determine_author_info();
+
 	if (use_editor) {
 		if (in_merge)
 			fprintf(fp,
@@ -545,6 +552,15 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 		if (only_include_assumed)
 			fprintf(fp, "# %s\n", only_include_assumed);
 
+		author_ident = xstrdup(fmt_name(author_name, author_email));
+		committer_ident =
+			xstrdup(fmt_name(getenv("GIT_COMMITTER_NAME"),
+					 getenv("GIT_COMMITTER_EMAIL")));
+		if (strcmp(author_ident, committer_ident))
+			fprintf(fp,
+				"# Author:    %s\n",
+				fmt_name(author_name, author_email));
+
 		saved_color_setting = wt_status_use_color;
 		wt_status_use_color = 0;
 		commitable = run_status(fp, index_file, prefix, 1);
@@ -920,7 +936,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		strbuf_addf(&sb, "parent %s\n", sha1_to_hex(head_sha1));
 	}
 
-	determine_author_info(&sb);
+	strbuf_addf(&sb, "author %s\n",
+		    fmt_ident(author_name, author_email, author_date, IDENT_ERROR_ON_NO_NAME));
 	strbuf_addf(&sb, "committer %s\n", git_committer_info(IDENT_ERROR_ON_NO_NAME));
 	if (!is_encoding_utf8(git_commit_encoding))
 		strbuf_addf(&sb, "encoding %s\n", git_commit_encoding);
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 284c941..95acf4c 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -154,6 +154,16 @@ test_expect_success 'cleanup commit messages (strip,-F,-e)' '
 
 '
 
+echo "# Author:    $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>" >> expect
+
+test_expect_success 'author different from committer' '
+
+	echo >>negative &&
+	git commit -e -m "sample"
+	head -n 5 .git/COMMIT_EDITMSG >actual &&
+	test_cmp expect actual
+'
+
 pwd=`pwd`
 cat >> .git/FAKE_EDITOR << EOF
 #! /bin/sh
-- 
1.5.5.1.148.g37726

[PATCH 3/3] commit: Show the committer ident when is different from the parent

[Santi Béjar]

To remind the committer ident in case it is not what you want
(taken from the gecos field, want to use different ident in
different repositories).

Signed-off-by: Santi Béjar <sbejar@gmail.com>
---
Hi,

  it does not work with the initial commit, I don't know why.
  Even with this:
	if (!commit)
		return NULL;

  Can someone help me? Thanks

 builtin-commit.c  |   30 ++++++++++++++++++++++++++++++
 t/t7502-commit.sh |   12 ++++++++++++
 2 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index c51c70f..06582ef 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -437,6 +437,29 @@ static void determine_author_info()
 	author_date = date;
 }
 
+const char *get_parent_ident()
+{
+	unsigned char sha1[20];
+	struct commit *commit;
+	const char *a, *lb, *rb, *eol;
+
+
+	get_sha1("HEAD", sha1);
+	commit = lookup_commit_reference(sha1);
+	if (!commit)
+		return NULL;
+
+	a = strstr(commit->buffer, "\ncommitter ");
+
+	lb = strstr(a + 11, " <");
+	rb = strstr(a + 11, "> ");
+	eol = strchr(a + 11, '\n');
+	if (!lb || !rb || !eol)
+		return NULL;
+
+	return xstrndup(a + 11, rb + 1 - (a + 11));
+}
+
 static int prepare_to_commit(const char *index_file, const char *prefix)
 {
 	struct stat statbuf;
@@ -448,6 +471,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 	const char *hook_arg2 = NULL;
 	const char *author_ident;
 	const char *committer_ident;
+	const char *parent_ident;
 
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		return 0;
@@ -561,6 +585,12 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 				"# Author:    %s\n",
 				fmt_name(author_name, author_email));
 
+		parent_ident = xstrdup(get_parent_ident());
+		if (strcmp(parent_ident, committer_ident))
+			fprintf(fp,
+				"# Committer: %s\n",
+				committer_ident);
+
 		saved_color_setting = wt_status_use_color;
 		wt_status_use_color = 0;
 		commitable = run_status(fp, index_file, prefix, 1);
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 95acf4c..96b5dcb 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -164,6 +164,18 @@ test_expect_success 'author different from committer' '
 	test_cmp expect actual
 '
 
+GIT_COMMITTER_NAME="C. O. Mitter"
+export GIT_COMMITTER_NAME
+echo "# Committer: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >> expect
+
+test_expect_success 'committer different from the parent committer' '
+
+	echo >>negative &&
+	git commit -e -m "sample"
+	head -n 6 .git/COMMIT_EDITMSG >actual &&
+	test_cmp expect actual
+'
+
 pwd=`pwd`
 cat >> .git/FAKE_EDITOR << EOF
 #! /bin/sh
-- 
1.5.5.1.148.g37726

Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent

[Jeff King]

On Wed, Apr 30, 2008 at 10:47:16AM +0200, Santi Béjar wrote:

>   it does not work with the initial commit, I don't know why.
>   Even with this:
> 	if (!commit)
> 		return NULL;
> 
>   Can someone help me? Thanks

It segfaults for me, since you try to strdup NULL. I think you want to
return "" if there is no initial commit, so you can strcmp against the
current committer (or alternatively, explicitly check for NULL).

> +		parent_ident = xstrdup(get_parent_ident());
> +		if (strcmp(parent_ident, committer_ident))
> +			fprintf(fp,
> +				"# Committer: %s\n",
> +				committer_ident);
> +

Why strdup at all here, which leaks? Nobody else uses this variable, so
it should be sufficient to strcmp(get_parent_ident(), committer_ident)
once get_parent_ident promises to always return a valid string. Or
alternatively:

  parent_ident = get_parent_ident();
  if (!parent_ident || strcmp(parent_ident, committer_ident))


A few other comments (I like the idea overall):

  - I haven't looked at the code organization, so maybe it is not
    feasible, but it seems like this stuff should go into wt-status's
    implementation, which would show up in a git-status.

  - The output looks very cluttered. It would be nice to have

    # enter your commit message...
    #
    # Author: whatever
    # Committer: whatever
    #
    # other stuff

    IOW, put in a blank line on either side, but not between the two
    identities if both are shown.

-Peff

Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent

[Jeff King]

On Wed, Apr 30, 2008 at 10:50:17AM -0400, Jeff King wrote:

> It segfaults for me, since you try to strdup NULL. I think you want to
> return "" if there is no initial commit, so you can strcmp against the
> current committer (or alternatively, explicitly check for NULL).
> [...]
>     IOW, put in a blank line on either side, but not between the two
>     identities if both are shown.

Fixes for these are below (no commit message; I think you should respin
the series rather than making this a separate patch). I didn't look at
putting it into wt-status.c, but I think that is worth pursuing.

---
 builtin-commit.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 30828c5..4e62510 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -472,6 +472,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 	const char *author_ident;
 	const char *committer_ident;
 	const char *parent_ident;
+	int showed_ident = 0;
 
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		return 0;
@@ -582,15 +583,20 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 					 getenv("GIT_COMMITTER_EMAIL")));
 		if (strcmp(author_ident, committer_ident))
 			fprintf(fp,
-				"# Author:    %s\n",
+				"%s# Author:    %s\n",
+				showed_ident++ ? "" : "#\n",
 				fmt_name(author_name, author_email));
 
-		parent_ident = xstrdup(get_parent_ident());
-		if (strcmp(parent_ident, committer_ident))
+		parent_ident = get_parent_ident();
+		if (!parent_ident || strcmp(parent_ident, committer_ident))
 			fprintf(fp,
-				"# Committer: %s\n",
+				"%s# Committer: %s\n",
+				showed_ident++ ? "" : "#\n",
 				committer_ident);
 
+		if (showed_ident)
+			fprintf(fp, "#\n");
+
 		saved_color_setting = wt_status_use_color;
 		wt_status_use_color = 0;
 		commitable = run_status(fp, index_file, prefix, 1);
-- 
1.5.5.1.183.g593d89.dirty

Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent

[Jeff King]

On Wed, Apr 30, 2008 at 10:50:17AM -0400, Jeff King wrote:

> A few other comments (I like the idea overall):

Actually, thinking about this a bit more (and using it), I'm not sure
that the best rule for showing the committer is "differs from the
previous committer." For example, if I am building on Junio's git.git,
the committer name is shown, even though I have made many commits in
this repo already.

So if the goal is to show the committer name only when it "matters", I
don't think you have succeeded.

-Peff

Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent

[Alex Riesen]

Santi Béjar, Wed, Apr 30, 2008 10:47:16 +0200:
> To remind the committer ident in case it is not what you want
> (taken from the gecos field, want to use different ident in
> different repositories).

Like: show committer id always if the repo is any kind of a public
project. Which maybe ok, I'd better always see what id I use to work
on, for example, Git.

I like the idea in general, but the implementation could be refined.
Maybe consider looking at user.name (whether the repo has an ident
set)? This is usually done conciously, and the user wont need this
reminder.

Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent

[Santi Béjar]

On Wed, Apr 30, 2008 at 6:37 PM, Jeff King <peff@peff.net> wrote:
>  Actually, thinking about this a bit more (and using it), I'm not sure
>  that the best rule for showing the committer is "differs from the
>  previous committer." For example, if I am building on Junio's git.git,
>  the committer name is shown, even though I have made many commits in
>  this repo already.
>
>  So if the goal is to show the committer name only when it "matters", I
>  don't think you have succeeded.
>
>  -Peff
>

On Wed, Apr 30, 2008 at 7:52 PM, Alex Riesen <raa.lkml@gmail.com> wrote:
>  I like the idea in general, but the implementation could be refined.
>  Maybe consider looking at user.name (whether the repo has an ident
>  set)? This is usually done conciously, and the user wont need this
>  reminder.

Maybe only show the committer ident when both happens:

1) the committer ident is different from the parent
2) the committer ident is set automatically

Santi

P.D.: Jeff, thanks for the amending patch.

Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent

[Jeff King]

On Wed, Apr 30, 2008 at 09:23:43PM +0200, Santi Béjar wrote:

> >  I like the idea in general, but the implementation could be refined.
> >  Maybe consider looking at user.name (whether the repo has an ident
> >  set)? This is usually done conciously, and the user wont need this
> >  reminder.
> 
> Maybe only show the committer ident when both happens:
> 
> 1) the committer ident is different from the parent
> 2) the committer ident is set automatically

Honestly, I think just "2)" is probably fine (where automatically
presumably means "from GECOS"). I see what you are trying to accomplish
with "1)", but it's so workflow specific as to be useless.

> P.D.: Jeff, thanks for the amending patch.

No problem.

-Peff

Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent

[Santi Béjar]

On Wed, Apr 30, 2008 at 9:26 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Apr 30, 2008 at 09:23:43PM +0200, Santi Béjar wrote:
>
>  > >  I like the idea in general, but the implementation could be refined.
>  > >  Maybe consider looking at user.name (whether the repo has an ident
>  > >  set)? This is usually done conciously, and the user wont need this
>  > >  reminder.
>  >
>  > Maybe only show the committer ident when both happens:
>  >
>  > 1) the committer ident is different from the parent
>  > 2) the committer ident is set automatically
>
>  Honestly, I think just "2)" is probably fine (where automatically
>  presumably means "from GECOS"). I see what you are trying to accomplish
>  with "1)", but it's so workflow specific as to be useless.

I don't see "1) as workflow specific, it is there to minimize the
number of time it is shown, ie somebody might prefer the automatic
ident, so as long as it does not change it is not shown.

Santi

Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent

[Jeff King]

On Thu, May 01, 2008 at 10:34:44AM +0200, Santi Béjar wrote:

> >  > 1) the committer ident is different from the parent
> >  > 2) the committer ident is set automatically
> >
> >  Honestly, I think just "2)" is probably fine (where automatically
> >  presumably means "from GECOS"). I see what you are trying to accomplish
> >  with "1)", but it's so workflow specific as to be useless.
> 
> I don't see "1) as workflow specific, it is there to minimize the
> number of time it is shown, ie somebody might prefer the automatic
> ident, so as long as it does not change it is not shown.

Yes, but whether it actually succeeds in minimizing is dependent on the
workflow of the user. I.e., it works great if I tend to build on small,
personal repositories (or topic branches), or if I tend to receive other
people's work by patches that I commit myself. But if I am pulling from
somebody else's repo (as many of us do from git.git), or if I use a
shared repo with the other committers, then I am very likely to be
building on a parent that was committed by somebody else (and
consequently will see the committer mentioned way more often than was
intended).

-Peff

Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, May 01, 2008 at 10:34:44AM +0200, Santi Béjar wrote:
>
>> >  > 1) the committer ident is different from the parent
>> >  > 2) the committer ident is set automatically
>> >
>> >  Honestly, I think just "2)" is probably fine (where automatically
>> >  presumably means "from GECOS"). I see what you are trying to accomplish
>> >  with "1)", but it's so workflow specific as to be useless.
>> 
>> I don't see "1) as workflow specific, it is there to minimize the
>> number of time it is shown, ie somebody might prefer the automatic
>> ident, so as long as it does not change it is not shown.
>
> Yes, but whether it actually succeeds in minimizing is dependent on the
> workflow of the user. I.e., it works great if I tend to build on small,
> personal repositories (or topic branches), or if I tend to receive other
> people's work by patches that I commit myself. But if I am pulling from
> somebody else's repo (as many of us do from git.git), or if I use a
> shared repo with the other committers, then I am very likely to be
> building on a parent that was committed by somebody else (and
> consequently will see the committer mentioned way more often than was
> intended).

The goal of the patch as I understand it is to prevent mistakes of
committing under a wrong committer ident, isn't it?  Why does "minimizing"
come into the picture?

Once you have a good algorithm to see when to trigger the warning that the
user might be using an unintended committer identity, I do not think you
should refrain from issuing the warning when you see the offending
committer ident and whose commit you are building on top of should not
affect it.  Otherwise, the user will get the warning once (or not even get
the warning because the commit was made with a "commit -s -m 'typofix'"
one liner), and keep using the wrong ident without noticing before
building up a long chain of commits.  If the identification algorithm is
bogus, that would result in too many false hits to be annoying, and in
that case, (1) I would not apply such a bogus algorithm until it gets
into reasonable shape, (2) we would improve it after it gets applied and
if still makes noise on a hopefully rarer false positive cases, and/or (3)
the warning will be accompanied with a suggestion to explicitly use
user.name and/or user.email in the configuration file to allow the user to
squelch it.

I think the other patch about showing the author when you are committing
other's changes is a good move, by the way.

Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent

[Jeff King]

On Thu, May 01, 2008 at 12:02:51PM -0700, Junio C Hamano wrote:

> Once you have a good algorithm to see when to trigger the warning that the
> user might be using an unintended committer identity, I do not think you
> should refrain from issuing the warning when you see the offending
> committer ident and whose commit you are building on top of should not
> affect it.  Otherwise, the user will get the warning once (or not even get

I think I must not be writing very clearly, because that is basically
the same point I have been trying to make in this thread.  Santi's
original algorithm for warning about the wrong committer ident was to
check whether it matched the parent commit. But my point was that is a
bad algorithm, because it has way too many false positives (i.e., you
will end up showing the committer _all the time_ in many workflows).
So the followup was "check this other thing, and also look at whether it
matches the parent" to which my response was "why bother checking the
parent match then?"

I also think you could argue that we should just show the committer all
the time. But I don't think anyone has made that argument.

> I think the other patch about showing the author when you are committing
> other's changes is a good move, by the way.

I agree; it has a very straightforward and sensible algorithm for when
to show.

-Peff

Re: [PATCH 3/3] commit: Show the committer ident when is different from the parent

[Santi Béjar]

On Fri, May 2, 2008 at 12:28 AM, Jeff King <peff@peff.net> wrote:
> On Thu, May 01, 2008 at 12:02:51PM -0700, Junio C Hamano wrote:
>
>  > Once you have a good algorithm to see when to trigger the warning that the
>  > user might be using an unintended committer identity, I do not think you
>  > should refrain from issuing the warning when you see the offending
>  > committer ident and whose commit you are building on top of should not
>  > affect it.  Otherwise, the user will get the warning once (or not even get
>
>  I think I must not be writing very clearly,

It was clear, but I think there is more than this, especially the
definition of what is a wrong committer depends on the user. I was
trying to not annoy too much to some users (those who like the
automatic committer).

Let's find the definition of "wrong committer":

1) user.{name,email} or GIT_COMMITTER_{NAME,EMAIL} is not a wrong committer.
2) automatic without a domain name (user@hostname.(none)) is a wrong committer.
2) automatic or partially set ident:
    a) wrong committer for some users
    b) right committer for others

I see different strategies. Show the committer:

1) always
2) when user.warn = yes (defaulting to yes)
3) when it is automatic
    a) always
    b) and different from parent
    c) and different from a list of valid committer idents
    d) and when user.WarnAutomatic = yes (defaulting to yes)

(the names of the configs are just ideas)

I prefer in order: 3a, 3d, 2, 1.

>
>  I also think you could argue that we should just show the committer all
>  the time. But I don't think anyone has made that argument.

>From past threads and the "precious screen state" I thought it was
totally discarded.

Santi