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

From: Ben Keene <seraphire@gmail.com>
Date: Dec 13, 2019, 13:42 UTC
Message-ID: <0afed92d-6efb-380e-cf02-d0a5d35e63a7@gmail.com>
In-Reply-To: <20191213014537.GA13064@generichostname>

On 12/12/2019 8:45 PM, Denton Liu wrote:

Show 13 quoted lines

> Hi Ben,
>
> On Thu, Dec 12, 2019 at 07:46:24PM +0000, Ben Keene via GitGitGadget wrote:
>> From: Ben Keene <seraphire@gmail.com>
>> ...
>>    * choices are extracted from the prompt text [.]
>>        a single letter surrounded by square brackets
>>        is selected as a valid choice.
> Maybe something like this?
>
> 	* returns a single character where valid return values are
> 	  found by inspecting prompt_text for single characters
> 	  surrounded by square brackets

Yes, that is much more readable.

Show 12 quoted lines

> ...
> Aside from the one comment at the bottom, I reviewed the rest of this
> patch with `-w` and it looks good to me. Unfortunately, I don't use or
> know p4 so I haven't tested it.
>
>> @@ -4170,3 +4175,4 @@ def main():
>>   
>>   if __name__ == '__main__':
>>       main()
>> +
> Spurious trailing line. Perhaps we could make GGG error out on
> whitespace errors before submissions are allowed?

That is a good idea. I obviously missed that and it would help if it reported the error before submission.

>> -- 
>> gitgitgadget
>>