Re: [PATCH v4 1/2] merge-file: let conflict markers match end-of-line style of the context

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 26, 2016, 18:23 UTC
Message-ID: <xmqqoac8ci8o.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <13379804ad901b53d78d741156677664924fc50d.1453819314.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

Show 14 quoted lines

> +static int is_cr_needed(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m)
> +{
> +	int needs_cr;
> +
> +	/* Match post-images' preceding, or first, lines' end-of-line style */
> +	needs_cr = is_eol_crlf(&xe1->xdf2, m->i1 ? m->i1 - 1 : 0);
> +	if (needs_cr)
> +		needs_cr = is_eol_crlf(&xe2->xdf2, m->i2 ? m->i2 - 1 : 0);
> +	/* Look at pre-image's first line, unless we already settled on LF */
> +	if (needs_cr)
> +		needs_cr = is_eol_crlf(&xe1->xdf1, 0);
> +	/* If still undecided, use LF-only */
> +	return needs_cr < 0 ? 0 : needs_cr;
> +}

Retrying with other images when needs_cr is either -1 (unknown) or 1 (known to be true) was tricky; I had to read it twice and think about it for 30 seconds before convincing myself that the code does what the log message specifies it should.

That is probably because I was thinking in terms of "do we know that we need to add a CR?"; if I read "needs_cr" in my head as "we might want to add a CR", everything becomes much more clear, but perhaps it is just me.

The return value of this function is definitely "do we need and want to add a CR", and it is appropriately named.

Thanks.