Re: [PATCH] send-email: add proper default sender

On Tue, Nov 13, 2012 at 12:42:02AM +0100, Felipe Contreras wrote:

quoted

Why not use Git::ident_person() here? It saves some code, and would also
respect environment variables. Or better yet...

I assume there was a reason why that code was asking for input;
precisely because it would use the environment variables. For some
reason the user might have exported GIT_AUTHOR_EMAIL, or maybe EMAIL
is not right, or the full name config.

OTOH user.name/.email configurations come clearly from the user.

But we use the environment to default the field, so the distinction
doesn't make much sense to me.  Plus, it has always been the case that
you can use git without setting user.*, but instead only using the
environment. I don't see any reason not to follow that principle here,
too.

The one distinction that would make sense to me is pausing to ask when
we use "implicit" methods to look up the ident, like concatenating the
username with the hostname to get the email.

Git::ident uses "git var" to do its lookup, which will use IDENT_STRICT;
that stops most junk like empty names and bogus domains. But I think we
would want to go one step further and actually check
user_ident_sufficiently_given.  Unfortunately, that is not currently
available outside of C. You'd probably want something like:

diff --git a/builtin/var.c b/builtin/var.c
index aedbb53..eaf324e 100644
--- a/builtin/var.c
+++ b/builtin/var.c

@@ -26,6 +26,12 @@ static const char *pager(int flag)
 	return pgm;
 }
 
+static const char *explicit_ident(int flag)
+{
+	git_committer_info(flag);
+	return user_ident_sufficiently_given() ? "1" : "0";
+}
+
 struct git_var {
 	const char *name;
 	const char *(*read)(int);

@@ -35,6 +41,7 @@ static struct git_var git_vars[] = {
 	{ "GIT_AUTHOR_IDENT",   git_author_info },
 	{ "GIT_EDITOR", editor },
 	{ "GIT_PAGER", pager },
+	{ "GIT_EXPLICIT_IDENT", explicit_ident },
 	{ "", NULL },
 };

-Peff