Re: [PATCH 1/5] add central method for prompting a user using GIT_ASKPASS or SSH_ASKPASS

Thomas Adam [off-list ref] writes:

On 27 December 2011 20:47, Junio C Hamano [off-list ref] wrote:

quoted

Sven Strickroth [off-list ref] writes:

quoted

+sub askpass_prompt {
+     my ($self, $prompt) = _maybe_self(@_);
+     if (exists $ENV{'GIT_ASKPASS'}) {
+             return _askpass_prompt($ENV{'GIT_ASKPASS'}, $prompt);
+     } elsif (exists $ENV{'SSH_ASKPASS'}) {
+             return _askpass_prompt($ENV{'SSH_ASKPASS'}, $prompt);
+     } else {
+             return undef;

Two problems with this if/elsif/else cascade.

 - If _askpass_prompt() fails to open the pipe to ENV{'GIT_ASKPASS'}, it
  will return 'undef' to us. Don't we want to fall back to SSH_ASKPASS in
  such a case?

 - The last "return undef" makes all callers of this method to implement a
  fall-back way somehow. I find it very likely that they will want to use

Not only that, "return undef" will have nasty side-effects if this
subroutine is called in list-context -- it's usually discouraged to
have explicit returns of "undef", where in scalar context that might
be OK, but in list context, the caller will see:

(undef)

and not:

()

i.e., the empty list.

Well, for this particular function whose interface is "I'll give you a
prompt, use it to interact with the user and give me what the user gave us
in response", a scalar caller would do

	my $response = askpass_prompt("What is your password?");

while a list context caller would instead do

	my ($response) = askpass_prompt("What is your password?");

or

	my @answer = askpass_prompt("What is your password?");
        my $response = $answer[0];

and all three callers would get "undef" in $response. I suspect returning
(undef) is a better thing to do, than relying that

        my @answer = ();
        my $response = $answer[0];

happes to give undef to $response because the access goes beyond the end
of the array, no?