Web lists-archives.com

Re: [PATCH 15/19] diff.c: convert diff_flush to use emit_line_*




On Mon, May 15, 2017 at 1:21 PM, Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote:
> On 05/13/2017 09:01 PM, Stefan Beller wrote:
>>
>> In a later patch, I want to propose an option to detect&color
>> moved lines in a diff, which cannot be done in a one-pass over
>> the diff. Instead we need to go over the whole diff twice,
>> because we cannot detect the first line of the two corresponding
>> lines (+ and -) that got moved.
>>
>> So to prepare the diff machinery for two pass algorithms
>> (i.e. buffer it all up and then operate on the result),
>> move all emissions to places, such that the only emitting
>> function is emit_line_0.
>>
>> This covers diff_flush.
>>
>> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
>> ---
>>  diff.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/diff.c b/diff.c
>> index 07041a35ad..386b28cf47 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -4873,7 +4873,9 @@ void diff_flush(struct diff_options *options)
>>                                   term, 1);
>>                         if (options->stat_sep) {
>>                                 /* attach patch instead of inline */
>> -                               fputs(options->stat_sep, options->file);
>> +                               emit_line(options, NULL, NULL,
>> +                                         options->stat_sep,
>> +                                         strlen(options->stat_sep));
>
>
> Same comment as patch 10 - is it OK that we now output a prefix too?

In this case, I would think it is ok. The stat_sep is only used in
"format-patch --attach" for example, which makes NO sense to
use in combination with --line-prefix.

(That is already broken; at least the line-prefix part, as we
do *not* prefix all the lines with the given prefix. That is because
stat_sep is a multiline string emitted, starting with '\n'.)

Thanks,
Stefan