Re: [PATCH 5/5] make askpass_prompt a global prompt method for asking users

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 27, 2011, 21:41 UTC
Message-ID: <7vty4l4rr8.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7vd3b967ql.fsf@alter.siamese.dyndns.org>

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

Show 7 quoted lines

> I *suspect* the difference is that you discarded that "return false at the
> end to let the caller do whatever they want" found in patch 1/5 and have
> the fallback inside the prompt() funtion now. And if that is the primary
> difference between the old "askpass_prompt()" and the new "prompt()", I
> tend to think that the series should be restructured to use the "prompt()"
> semantics from the beginning. No reason to start with a known-to-be-wrong
> way to do a thing and then fix it in a series that is new to the codebase.

After reading the series again, I think the right structure of this patch series should be more like this:

 (1/3) Add Git->prompt($prompt) and make _read_password in git-svn.perl to
       use it. The prompt method should implement the Term::ReadKey based
       fallback, so that _read_password do not have to roll its own. IOW,
       a squash of your 1/5, 2/5, and a part of 5/5, plus possibly 4/5.

 (2/3) Enhance Git->prompt($prompt, $is_password), and convert the various
       existing terminal interacters to use it. The fallback in the prompt
       method, when it is not reading in a noecho mode, should read a
       single line from the standard input in cooked mode like your 5/5
       does. IOW, a squash of your 3/5 and a part of 5/5.

 (3/3) Possibly tests and docs.

Thanks.