From: Ben Keene Date: Tue, 10 Dec 2019 14:26:22 GMT Subject: Re: [PATCH 1/3] git-p4: [usability] yes/no prompts should sanitize user text Message-ID: <179dd921-d9d0-d26d-33e9-3664bf97fcc2@gmail.com> In-Reply-To: On 12/9/2019 5:00 PM, Junio C Hamano wrote: > "Ben Keene via GitGitGadget" writes: > >> From: Ben Keene >> >> 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 >> --- >> >> +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?