Re: [PATCH 07/19] diff.c: convert fn_out_consume to use emit_line_*
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 16, 2017, 01:00 UTC
- Message-ID
- <xmqqtw4l4nuk.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170514040117.25865-8-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
Show 46 quoted lines
> 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 the parts of fn_out_consume.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index aef159a919..93343a9ccc 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1289,7 +1289,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
> const char *context = diff_get_color(ecbdata->color_diff, DIFF_CONTEXT);
> const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
> struct diff_options *o = ecbdata->opt;
> - const char *line_prefix = diff_line_prefix(o);
>
> o->found_changes = 1;
>
> @@ -1301,14 +1300,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
>
> if (ecbdata->label_path[0]) {
> const char *name_a_tab, *name_b_tab;
> -
> name_a_tab = strchr(ecbdata->label_path[0], ' ') ? "\t" : "";
> name_b_tab = strchr(ecbdata->label_path[1], ' ') ? "\t" : "";
> -
> - 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? emit_line_fmt() receives o so it is possible diff_line_prefix(o) may be called there and prepended to the output over there, but I somehow do not think that is the case---in fact 06/19 does not seem to teach emit_line_fmt() to do something like that.
Unless emit_line() is now taught to do the line_prefix thing, that is.
But then the hunk we see below, which didn't add line_prefix to the output, would add an unwanted prefix, so that is not likely.
Hmph...
Show 12 quoted lines
> 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);
> } else {
> /*
> * Skip the prefix character, if any. With