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

From: Stefan Beller <sbeller@google.com>
Date: May 16, 2017, 16:23 UTC
Message-ID: <CAGZ79ka3JU=7WgF1tYTTam32QhZVokJxdqyOi33P-sd=XrUecw@mail.gmail.com>
In-Reply-To: <xmqqpof94nn2.fsf@gitster.mtv.corp.google.com>

On Mon, May 15, 2017 at 6:05 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 29 quoted lines

> Junio C Hamano <gitster@pobox.com> 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