Re: [PATCH 1/2] apply: guard against renames of non-existant empty files

From: René Scharfe <l.s.r@web.de>
Date: Feb 27, 2017, 22:18 UTC
Message-ID: <ed46f675-559a-88a3-cf97-d0ba7cf3112f@web.de>
In-Reply-To: <xmqqinnvwg2d.fsf@gitster.mtv.corp.google.com>

Am 27.02.2017 um 21:10 schrieb Junio C Hamano:

Show 19 quoted lines

> René Scharfe <l.s.r@web.de> writes:
>
>> Would it make sense to mirror the previously existing condition and
>> check for is_new instead?  I.e.:
>>
>> 			if ((!patch->is_delete && !patch->new_name) ||
>> 			    (!patch->is_new    && !patch->old_name)) {
>>
>
> Yes, probably.
>
>> or
>>
>> 			if (!(patch->is_delete || patch->new_name) ||
>> 			    !(patch->is_new    || patch->old_name)) {
>
> This happens after calling parse_git_header() so we should know the
> actual value of is_delete and is_new by now (instead of mistaking
> -1 aka "unknown" as true), so this rewrite would also be OK.

The two variants are logically equivalent -- (!a && !b) == !(a || b). I wonder if the second one may be harder to read, though.

René