Web lists-archives.com

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




Stefan Beller <sbeller@xxxxxxxxxx> writes:

> 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@xxxxxxxxxx>
> ---
>  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...

>  		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