Re: [PATCH v3] diff: ensure consistent diff behavior with ignore options

[Lidong Yan <yldhome2d2@gmail.com>]

Junio C Hamano <gitster@pobox.com> writes:
> 
> Lidong Yan <yldhome2d2@gmail.com> writes:
> 
>> +static int quick_consume(void *priv, char *line, unsigned long len)
>> +{
>> + struct emit_callback *ecbdata = priv;
>> + struct diff_options *o = ecbdata->opt;
>> +
>> + o->found_changes = 1;
>> + return 1;
>> +}
> 
> "make DEVELOPER=YesPlease" would not be very happy, without
> 
> -static int quick_consume(void *priv, char *line, unsigned long len)
> +static int quick_consume(void *priv, char *line UNUSED, unsigned long len UNUSED)

(I accidentally sent an unfinished email to Junio. Sorry about that.)
Another a good thing to test before commit. Thanks for the reminder.

>> static void diff_flush_stat(struct diff_filepair *p, struct diff_options *o,
>>    struct diffstat_t *diffstat)
>> {
>> @@ -6778,7 +6810,19 @@ void diff_flush(struct diff_options *options)
>>     DIFF_FORMAT_CHECKDIFF)) {
>> for (i = 0; i < q->nr; i++) {
>> struct diff_filepair *p = q->queue[i];
>> - if (check_pair_status(p))
>> + int need_flush = 1;
>> +
>> + if (!check_pair_status(p))
>> + continue;
>> +
>> + if (options->flags.diff_from_contents) {
>> + if (diff_flush_patch_quiet(p, options))
>> + need_flush = 1;
>> + else
>> + need_flush = 0;
>> + }
>> +
>> + if (need_flush)
>> flush_one_pair(p, options);
>> }
> 
> I am having a hard time understanding the logic in this loop.  Is it
> equivalent to the following loop, and ...
> 
> for (i = 0; i < q->nr; i++) {
> struct diff_filepair *p = q->queue[i];
> 
> if (!check_pair_status(p))
> continue;
> if (options->flags.diff_from_contents &&
>    !diff_flush_patch_quietly(p, options))
> continue; /* no changes */
> 
> flush_one_pair(p, options);
> }
> 
> ... if so, isn't the above rewrite easier to follow?

Yes, I was being stupid again. That’s obviously better.

Thanks, will send v4 soon.
Lidong