Re: [PATCH] git-send-email: ssh/login style password requests

[Junio C Hamano <gitster@pobox.com>]

Michael Witten <mfwitten@MIT.EDU> writes:

> +--smtp-pass::
> +	Password for SMTP-AUTH. The default value can be specified
> +	with the configuration variable 'sendemail.smtppass' or
> +	sendemail.<identity>.smtppass (see sendemail.identity).
> +	The argument is optional: If no argument is specified, then
> +	the password is assumed to be the empty string.
> +	If a username has been set, but no password has been set,
> +	the user is prompted for a password with masked input for
> +	privacy; passwords need not be recorded in configuration
> +	files.

I am a bit puzzled about the above description, though.  It is
not clear if there is any difference between "the empty string"
password, and "no password has been set".  It makes me wonder
what the possible cases are.

 (1) no *.smtppass configuration present; no --smtp-pass parameter
     is given on the command line.

 (2) *.smtppass configuration present but is an empty string; no
     --smtp-pass parameter is given on the command line.

 (3) *.smtppass configuration present with a non-empty string; no
     --smtp-pass parameter is given on the command line.

 (4) parameter is given with an empty string (i.e. "--smtp-pass=")
     on the command line.

 (5) parameter is given with a non-empty string (i.e. "--smtp-pass=foo")
     on the command line.

My _guess_ is that command line always trump configuration so
in cases (4) and (5) it does not matter what you have (or do not
have) in *.smtppass configuration.  In other cases, lack of *.smtppass
and having *.smtppass with an empty string as its value are equivalent.
And in any case, an empty string (or lack of specification) results
in prompting.

But you shouldn't make _me_ guess.  You certainly would be
forcing many other people to guess with this.

> +		if (defined $smtp_authuser) {
> +
> +			if (!defined $smtp_authpass) {
> +				
> +				system "stty -echo";
> +
> +				do {
> +					$_ = $term->readline("Password: ");
> +				} while (!defined $_);
> +
> +				system "stty echo";
> +
> +				$smtp_authpass = $_ if ($_);
> +			}
> +

I like what the patch tries to do, but the system() there feels
a tad ugly.  In addition it makes me wonder (1) what happens if
you ^C out from this while loop, and (2) $term->readline()
interface might already have a method to turn echo off.

We luckily have people handy at Perl on the list, so I'll wait
for our resident Perl experts to suggest a better alterantive.

We are not in a hurry, so I expect a polished resubmit after
1.5.4.  Thanks.