From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 21 Jun 2017 20:09:45 GMT
Subject: Re: [PATCH 11/26] diff.c: emit_diff_symbol learns DIFF_SYMBOL_FILEPAIR
Message-ID: <xmqqvanpnlva.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170620130147.75e1fb78@twelve2.svl.corp.google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

> On Mon, 19 Jun 2017 19:48:01 -0700
> Stefan Beller <sbeller@google.com> wrote:
>
>> @@ -676,6 +677,14 @@ static void emit_diff_symbol(struct diff_options *o, enum diff_symbol s,
>>  		}
>>  		emit_line(o, context, reset, line, len);
>>  		break;
>> +	case DIFF_SYMBOL_FILEPAIR:
>> +		meta = diff_get_color_opt(o, DIFF_METAINFO);
>> +		reset = diff_get_color_opt(o, DIFF_RESET);
>> +		fprintf(o->file, "%s%s%s%s%s%s\n", diff_line_prefix(o), meta,
>> +			flags ? "+++ " : "--- ",
>
> I think that a constant should be defined for this flag, just like for
> content lines. If not it is not immediately obvious that a flag should
> be set for the +++ lines.

True.

>> +			line, reset,
>> +			strchr(line, ' ') ? "\t" : "");
>> +		break;

Also this is somewhat misnamed in that a call to it only deals with
one half of a filepair, iow, the caller must call it twice to show a
single filepair.