From: Jeff King <peff@peff.net>
Date: Mon, 28 Apr 2014 19:23:49 GMT
Subject: Re: [PATCH] Uses git-credential for git-imap-send
Message-ID: <20140428192349.GC25993@sigill.intra.peff.net>
In-Reply-To: <907d1444a8e17a43c03ee0c2bb04038bdd3372b8.1398620231.git.danalbert@google.com>

On Sun, Apr 27, 2014 at 10:58:51AM -0700, Dan Albert wrote:

> git-imap-send was directly prompting for a password rather than using
> git-credential. git-send-email, on the other hand, supports git-credential.
> 
> This is a necessary improvement for users that use two factor authentication, as
> they should not be expected to remember all of their app specific passwords.
> 
> Signed-off-by: Dan Albert <danalbert@google.com>

Thanks, this version looks good to me.

I noticed that we are just filling in the password here, since we'll
always fill cred.username from srvc->user. The lines directly above are:

	if (!srvc->user) {
		fprintf(stderr, "Skipping server %s, no user\n", srvc->host);
		goto bail;
	}

That comes from the imap.user config variable. I wonder if we should
just pass it off to credential_fill() in this case, too, which will fill
in the username if necessary.

It probably doesn't matter much, though, as nobody is complaining. And
if we were designing from scratch, I would say that "imap.user" and
"imap.pass" would not need to exist, as you can configure
"credential.imaps://host/.*" for the same purpose. But since we would
have to keep supporting them anyway for compatibility, it's not worth
trying to transition.

-Peff