"Ben Keene via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> From: Ben Keene <seraphire@xxxxxxxxx>
>
> Issue: when prompting the user with raw_input, the tests are not forgiving of user input. For example, on the first query asks for a yes/no response. If the user enters the full word "yes" or "no" the test will fail. Additionally, offer the suggestion of setting git-p4.attemptRCSCleanup when applying a commit fails because of RCS keywords. Both of these changes are usability enhancement suggestions.
Drop "Issue: " and upcase "when" that follows. The rest of the
paragraph reads a lot better without it as a human friendly
description.
"are usability enhancement suggestions"??? Leaves readers wonder
who suggested them, or you are suggesting but are willing the change
to be dropped, or what. Be a bit more assertive if you want to say
that you believe these two would improve usability.
> Change the code prompting the user for input to sanitize the user input before checking the response by asking the response as a lower case string, trimming leading/trailing spaces, and returning the first character.
>
> Change the applyCommit() method that when applying a commit fails becasue of the P4 RCS Keywords, the user should consider setting git-p4.attemptRCSCleanup.
s/becasue/because/;
I have a feeling that these two may be worth doing but are totally
separate issues, deserving two separate commits. Is there a good
reason why these two must go hand-in-hand?
> Signed-off-by: Ben Keene <seraphire@xxxxxxxxx>
> (cherry picked from commit 1fab571664f5b6ad4ef321199f52615a32a9f8c7)
> ---
> git-p4.py | 31 ++++++++++++++++++++++++++-----
> 1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/git-p4.py b/git-p4.py
> index f7c0ef0c53..f13e4645a3 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1909,7 +1909,8 @@ def edit_template(self, template_file):
> return True
>
> while True:
> - response = raw_input("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ")
> + response = raw_input("Submit template unchanged. Submit anyway? [y]es, [n]o (skip this patch) ").lower() \
> + .strip()[0]
You could have saved the patch by doing
+ .lower().strip()[0]
instead, no?
I wonder if it would be better to write a thin wrapper around raw_input()
that does the "downcase and take the first meaningful letter" thing
for you and call it prompt() or something like that.
> @@ -4327,7 +4343,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
> +
This change may be a good idea to give help text when the command
line parsing fails, but a good change deserves to be explained. I
do not think I saw any mention of it in the proposed log message,
though.
> global verbose
> verbose = cmd.verbose
> if cmd.needsGit:
