Sven Strickroth <sven.strickroth@xxxxxxxxxxxxxxx> writes:
> +=item prompt ( PROMPT)
> +
> +Checks if GIT_ASKPASS or SSH_ASKPASS is set, and if yes
> +use it and return answer from user.
> +
> +=cut
I think it would be good idea to describe what this function is for...
> +sub prompt {
> + my ($self, $prompt) = _maybe_self(@_);
> + if (exists $ENV{'GIT_ASKPASS'}) {
> + return _prompt($ENV{'GIT_ASKPASS'}, $prompt);
> + } elsif (exists $ENV{'SSH_ASKPASS'}) {
> + return _prompt($ENV{'SSH_ASKPASS'}, $prompt);
> + } else {
> + return undef;
> + }
> +}
...and provide some kind of fallback even if neither of GIT_ASKPASS
nor SSH_ASKPASS are set (perhaps assuming that some Perl packages from
CPAN are installed).
> +sub _prompt {
> + my ($self, $askpass, $prompt) = _maybe_self(@_);
> + my $ret;
> + open(PH, "-|", $askpass, $prompt);
> + $ret = <PH>;
> + $ret =~ s/[\012\015]//g; # strip \n\r
> + close(PH);
> + return $ret;
> +}
Please, use modern Perl, in particula use lexical filehandles instead
of typeglobs (which are global variables), i.e.
+ open my $fh, "-|", $askpass, $prompt
+ or die "...";
+ $ret = <$fh>;
+ chomp($ret);
+ close($fh)
+ or die "...";
> --
> 1.7.7.1.msysgit.0
>
> From ef4c6557d1b0e33440d13c64742d44b2a22143f3 Mon Sep 17 00:00:00 2001
> From: Sven Strickroth <email@xxxxxxxxxx>
> Date: Tue, 27 Dec 2011 00:34:09 +0100
> Subject: [PATCH 2/4] switch to central prompt method
>
> Signed-off-by: Sven Strickroth <email@xxxxxxxxxx>
Please send those patches as a separate emails, not concatenated in a
single email (perhaps even with cover letter).
See Documentation/SubmittingPatches
[...]
--
Jakub Narebski
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html