Re: [PATCH v1 0/1] t6038-merge-text-auto.sh

From: Junio C Hamano <gitster@pobox.com>
Date: May 30, 2016, 18:48 UTC
Message-ID: <xmqqy46r8jre.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqzir7o286.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 11 quoted lines

> Imagine that you started from a history where somebody recorded a
> text file with CRLF in the blob, unconverted.  Later the project
> decided to express their text with LF to support cross-platform
> development better, and sets up the Auto-CRLF.  Your user is working
> near the tip of that history after the eol correction happened.  Now
> she gets a pull-request of a branch that forked from an old point in
> the history, before the eol correction and full of CRLF.  Yes, to
> integrate the change being proposed, she needs to look at "theirs";
> that's the whole point of a "merge".  Why should she revert the eol
> correction her history has by getting fooled by the fact that the
> update was based on a part of the history before the eol correction?

Thinking aloud along this line a bit further, if you really cared (and I don't feel very strongly, as I think "safe crlf" is an ugly workaround that lets users keep their misconfiguration by penalizing their day-to-day operation), you may want to think about doing a 3-way merge of "eol preference" beween all stages.

That is, if the common ancestor in stage #1 and the current version in stage #2 both have its text in LF, and the data being merged in stage #3 is in CRLF, you sayr "do not convert; the change being brought in wants to have CRLF endings, while our history did not care". Similarly, if the common ancestor in stage #1 and the data being merged in stage #3 both have CRLF, and your version in stage #2 has LF, you say "We wanted to fix eol since the side branch forked, and the side branch predates that fix, so we keep the eol fix we did since we diverged", i.e. "Do convert".

For doing this, you may want to refactor the codepath that decides "Auto-CRLF usually wants to turn CRLF in text to LF, but should we disable that logic now, because the user already has CRLF in the current one?" into a function that takes a single parameter, `path`, and returns "Yes, do convert CRLF to LF / No, do not convert" boolean. Having a low level function that says "What's the blob at this path in the index" and have the caller run that logic feels unwieldy if we want to go that route.