Web lists-archives.com

Re: [PATCH 07/19] diff.c: convert fn_out_consume to use emit_line_*




On Mon, May 15, 2017 at 6:05 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>>> -            fprintf(o->file, "%s%s--- %s%s%s\n",
>>> -                    line_prefix, meta, ecbdata->label_path[0], reset, name_a_tab);
>>> -            fprintf(o->file, "%s%s+++ %s%s%s\n",
>>> -                    line_prefix, meta, ecbdata->label_path[1], reset, name_b_tab);
>>> +            emit_line_fmt(o, meta, reset, "--- %s%s\n",
>>> +                          ecbdata->label_path[0], name_a_tab);
>>> +            emit_line_fmt(o, meta, reset, "+++ %s%s\n",
>>> +                          ecbdata->label_path[1], name_b_tab);
>>
>> How is the loss of line_prefix from this call site compensated?
>
> OK, emit_line_0() has already been aware of line_prefix, so that is
> how the loss of line_prefix in the above is accounted for.  We are
> good here.
>
>>>              ecbdata->label_path[0] = ecbdata->label_path[1] = NULL;
>>>      }
>>>
>>> @@ -1349,7 +1346,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
>>>              diff_words_flush(ecbdata);
>>>              if (ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) {
>>>                      emit_line(o, context, reset, line, len);
>>> -                    fputs("~\n", o->file);
>>> +                    emit_line(o, NULL, NULL, "~\n", 2);
>
> So unless we have some magic here, we would see an extra line-prefix
> before that "~\n" thing, no?

Right.

With all the discussion on the the convert* patches, I consider just introducing
another parameter to emit_line, which specifies whether we want
emit_line to output
a line_prefix. Then the conversion becomes a lot easier as well.

Thanks,
Stefan