Re: [PATCH v2] add-patch: enforce only one-letter response to prompts

From: Junio C Hamano <gitster@pobox.com>
Date: May 22, 2024, 16:27 UTC
Message-ID: <xmqqv8356ccb.fsf@gitster.g>
In-Reply-To: <Zk3R4MuCWOYVz3_B@tanuki>

Patrick Steinhardt <ps@pks.im> writes:

Show 5 quoted lines

>> +		/* 'g' takes a hunk number, '/' takes a regexp */
>> +		if (1 < s->answer.len && (ch != 'g' && ch != '/')) {
>
> I find this condition a bit unusual and thus hard to read. If it instead
> said `s->answer.len != 1` then it would be way easier to comprehend.

We have already eliminated the "it is 0" case, .len cannot be negative, and the case we really care about is "is it not just one?", so I agree with you that the inequality comparison with 1 is easier to grok.

> I assume we also want a test for this new behaviour, right?

Hmph, yeah. 'g' has already been tested (that was what led me to do the v2), but we probably should do 'qa' or something.

Thanks.