Sven Strickroth <sven.strickroth@xxxxxxxxxxxxxxx> writes:
> @@ -4357,11 +4357,10 @@ sub ssl_server_trust {
> issuer_dname fingerprint);
> my $choice;
> prompt:
> - print STDERR $may_save ?
> + my $options = $may_save ?
> "(R)eject, accept (t)emporarily or accept (p)ermanently? " :
> "(R)eject or accept (t)emporarily? ";
> - STDERR->flush;
> - $choice = lc(substr(<STDIN> || 'R', 0, 1));
> + $choice = substr(Git->prompt("Certificate unknown. " . $options) || 'R', 0, 1);
I am afraid the extra "Certificate unknown. " prefix may make the prompt
way too long to fit on a line on the terminal or in the GUI. Would it be
Ok to perhaps add LF to make it a multi-line prompt? Do GUI based helpers
make that into a dialog box with multi-line prompt, or do they just barf?
> if ($choice =~ /^t$/i) {
> $cred->may_save(undef);
We seem to have lost lc() there, but it probably is deliberate and
harmless, as the value is checked with /^x$/i later.
As we are making sure $choice has a single character anyway, I think that
checking with "=~ /^x$/i" is unnecessarily ugly and wrong, even though it
is obviously not the fault of this patch.
> @@ -4378,10 +4377,7 @@ prompt:
> sub ssl_client_cert {
> my ($cred, $realm, $may_save, $pool) = @_;
> $may_save = undef if $_no_auth_cache;
> - print STDERR "Client certificate filename: ";
> - STDERR->flush;
> - chomp(my $filename = <STDIN>);
> - $cred->cert_file($filename);
> + $cred->cert_file(Git->prompt("Client certificate filename: "));
> $cred->may_save($may_save);
> $SVN::_Core::SVN_NO_ERROR;
> }
We may later add an option to "prompt" method to allow the caller to say
that we are asking for a filename, and let GUI prompt helper to run a file
picker, but I think that is outside the immediate scope of this patch.
Just a thing for the future to keep in mind.
This patch itself looks almost perfect to me (modulo the above minor
nits), and except that it textually depends on 1/2 that may need to be
updated.
Thanks.
--
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