Re: [RFC/PATCH 05/17] diff.c: emit_line_0 can handle no color setting

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 13, 2016, 22:51 UTC
Message-ID: <xmqqy42vtnub.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160913044613.1037-6-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 14 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.
>
> In later patches we may pass lines that are not colored to
> the central function emit_line_0, so we
> need to emit the color only when it is non-NULL.

Explained this way, a reader would find that this step is here before the underlying code is ready--we are not even buffering at this step yet.

But that is OK. It used to be that passing "" as set/reset was the way to get a --no-color output. Now you can pass NULL instead of empty strings. That would be an alternative explanation why this is an acceptable change (as your later step probably has a good reason why it cannot pass "" instead of NULL).

Show 26 quoted lines

>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  diff.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index b6a40ae..5d57130 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -473,11 +473,13 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
>  	}
>  
>  	if (len || !nofirst) {
> -		fputs(set, file);
> +		if (set)
> +			fputs(set, file);
>  		if (!nofirst)
>  			fputc(first, file);
>  		fwrite(line, len, 1, file);
> -		fputs(reset, file);
> +		if (reset)
> +			fputs(reset, file);
>  	}
>  	if (has_trailing_carriage_return)
>  		fputc('\r', file);