From e2aed5fd5bfb0381a6f69373c01ef09d70467a5c Mon Sep 17 00:00:00 2001 From: Ben Keene Date: Mon, 16 Dec 2019 14:02:19 +0000 Subject: [PATCH 1/2] git-p4: yes/no prompts should sanitize user text 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) where * prompt_text is the text prompt for the user * returns a single character where valid return values are found by inspecting prompt_text for single characters surrounded by square brackets 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. Since the method requires the returned text to be one of the available choices, remove the loop from the calling code that handles response verification. Thanks-to: Denton Liu Signed-off-by: Ben Keene Signed-off-by: Junio C Hamano --- git-p4.py | 65 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/git-p4.py b/git-p4.py index 60c73b6a37..3b3f1469a6 100755 --- a/git-p4.py +++ b/git-p4.py @@ -167,6 +167,21 @@ def die(msg): sys.stderr.write(msg + "\n") sys.exit(1) +def prompt(prompt_text): + """ Prompt the user to choose one of the choices + + Choices are identified in the prompt_text by square brackets around + a single letter option. + """ + choices = set(m.group(1) for m in re.finditer(r"\[(.)\]", prompt_text)) + while True: + response = raw_input(prompt_text).strip().lower() + if not response: + continue + response = response[0] + if response in choices: + return response + def write_pipe(c, stdin): if verbose: sys.stderr.write('Writing pipe: %s\n' % str(c)) @@ -1778,12 +1793,11 @@ class P4Submit(Command, P4UserMap): if os.stat(template_file).st_mtime > mtime: return True - while True: - response = raw_input("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ") - if response == 'y': - return True - if response == 'n': - return False + response = prompt("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ") + if response == 'y': + return True + if response == 'n': + return False def get_diff_description(self, editedFiles, filesToAdd, symlinks): # diff @@ -2345,31 +2359,22 @@ class P4Submit(Command, P4UserMap): " --prepare-p4-only") break if i < last: - quit = False - while True: - # prompt for what to do, or use the option/variable - if self.conflict_behavior == "ask": - print("What do you want to do?") - response = raw_input("[s]kip this commit but apply" - " the rest, or [q]uit? ") - if not response: - continue - elif self.conflict_behavior == "skip": - response = "s" - elif self.conflict_behavior == "quit": - response = "q" - else: - die("Unknown conflict_behavior '%s'" % - self.conflict_behavior) + # prompt for what to do, or use the option/variable + if self.conflict_behavior == "ask": + print("What do you want to do?") + response = prompt("[s]kip this commit but apply the rest, or [q]uit? ") + elif self.conflict_behavior == "skip": + response = "s" + elif self.conflict_behavior == "quit": + response = "q" + else: + die("Unknown conflict_behavior '%s'" % + self.conflict_behavior) - if response[0] == "s": - print("Skipping this commit, but applying the rest") - break - if response[0] == "q": - print("Quitting") - quit = True - break - if quit: + if response == "s": + print("Skipping this commit, but applying the rest") + if response == "q": + print("Quitting") break chdir(self.oldWorkingDirectory) From 608e380502f754b50ead6c7e6c3f5ff5ee8eca33 Mon Sep 17 00:00:00 2001 From: Ben Keene Date: Mon, 16 Dec 2019 14:02:20 +0000 Subject: [PATCH 2/2] git-p4: show detailed help when parsing options fail When a user provides invalid parameters to git-p4, the program reports the failure but does not provide the correct command syntax. Add an exception handler to the command-line argument parser to display the command's specific command line parameter syntax when an exception is thrown. Rethrow the exception so the current behavior is retained. Signed-off-by: Ben Keene Signed-off-by: Junio C Hamano --- git-p4.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index 3b3f1469a6..9165ada2fd 100755 --- a/git-p4.py +++ b/git-p4.py @@ -4145,7 +4145,12 @@ def main(): description = cmd.description, formatter = HelpFormatter()) - (cmd, args) = parser.parse_args(sys.argv[2:], cmd); + try: + (cmd, args) = parser.parse_args(sys.argv[2:], cmd); + except: + parser.print_help() + raise + global verbose verbose = cmd.verbose if cmd.needsGit: