Re: [PATCH 2/2] git-svn, perl/Git.pm: extend and use Git->prompt method for querying users

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 28, 2011, 21:38 UTC
Message-ID: <7vpqf8z8a6.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7v39c41keo.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 9 quoted lines

> I actually was hoping that the answer is "it depends on the helper
> specified by *_ASKPASS".
>
> In any case, let's not add that extra "Certificate unknown. " prefix at
> all to avoid regressions and queue this patch series for real.
>
> After somebody comes up with a way to deal with overlong prompt, building
> on top of this series, we can work on making this particular prompt longer
> and more descriptive.

I've queued the two patches with minor tweaks.

I think the first patch is a definite improvement for both GUI users and terminal users who use the *_ASKPASS environment variable. Other parts of git already asks the latter their password using *_ASKPASS anyway, so I do not foresee complaints from them saying that git-svn suddenly stopped reading the password from the terminal.

I am however not sure if the second patch in this series is a good thing in the current shape. For GUI users who do not have a terminal, earlier they couldn't respond to these questions but now they can, so in that narrow sense we are not going backwards.

But for people who use *_ASKPASS and are working from the terminal, it is a regression to ask these non-password questions using *_ASKPASS. Most likely, these helpers that are designed for password entry will hide what is typed, and I also wouldn't be surprised if some of them have fairly low input-length restriction that may be shorter than a long-ish pathname that users might want to give as an answer, which they could do in the terminal based interaction but will become impossible with this patch.

I suspect that we would need to enhance *_ASKPASS interface first, so that we can ask things other than passwords. Until that happens, I do not think we should apply the second patch to use *_ASKPASS for non-passwords.