Re: [PATCH 1/3] git-p4: [usability] yes/no prompts should sanitize user text

[Ben Keene <seraphire@gmail.com>]

On 12/9/2019 5:00 PM, Junio C Hamano wrote:
> "Ben Keene via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Ben Keene <seraphire@gmail.com>
>>
>> When prompting the user interactively for direction, the tests are
>> not forgiving of user input format.
>>
>> For example, the first query asks for a yes/no response. If the user
>> enters the full word "yes" or "no" or enters a capital "Y" the test
>> will fail.
>>
>> Create a new function, prompt(prompt_text, choices) where
>>    * promt_text is the text prompt for the user
>>    * is a list of lower-case, single letter choices.
>> This new function must  prompt the user for input and sanitize it by
>> converting the response to a lower case string, trimming leading and
>> trailing spaces, and checking if the first character is in the list
>> of choices. If it is, return the first letter.
>>
>> Change the current references to raw_input() to use this new function.
>>
>> Signed-off-by: Ben Keene <seraphire@gmail.com>
>> ---
>>
>> +def prompt(prompt_text, choices = []):
>> +    """ Prompt the user to choose one of the choices
>> +    """
>> +    while True:
>> +        response = raw_input(prompt_text).strip().lower()
>> +        if len(response) == 0:
>> +            continue
>> +        response = response[0]
>> +        if response in choices:
>> +            return response
> I think this is a strict improvement compared to the original, but
> the new loop makes me wonder if we need to worry more about getting
> EOF while calling raw_input() here.  I am assuming that we would get
> EOFError either way so this is no worse/better than the status quo,
> and we can keep it outside the topic (even though it may be a good
> candidate for a low-hanging fruit for newbies).
That is a good catch.  What should we expect the default behavior
to be in these two questions if the EOFError occurs?  I would think
that we should extend this to an abort of the process?
> response = prompt("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ", ["y", "n"])
> response = prompt("[s]kip this commit but apply the rest, or [q]uit? ", ["s", "q"])

Should a quit be added to the first prompt and have those be the 
defaults on EOFError?