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

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 2811 bytes --]

Hi Ramsay,

On Mon, 25 Jan 2016, Ramsay Jones wrote:

> On 25/01/16 06:53, Johannes Schindelin wrote:
> > 
> > On Sun, 24 Jan 2016, Torsten Bögershausen wrote:
> > 
> >> On 24.01.16 11:48, Johannes Schindelin wrote:
> >> (I had the same reasoning about the CRLF in the working tree:
> >> We don't need to look at core.autocrlf/attributes, so Ack from me)
> >>
> >>> +test_expect_success 'conflict markers match existing line endings' '
> >>> +	append_cr <nolf-orig.txt >crlf-orig.txt &&
> >>> +	append_cr <nolf-diff1.txt >crlf-diff1.txt &&
> >>> +	append_cr <nolf-diff2.txt >crlf-diff2.txt &&
> >>> +	test_must_fail git -c core.eol=crlf merge-file -p \
> >>> +		crlf-diff1.txt crlf-orig.txt crlf-diff2.txt >crlf.txt &&
> >>> +	test $(tr "\015" Q <crlf.txt | grep "\\.txtQ$" | wc -l) = 3 &&
> >>> +	test_must_fail git -c core.eol=crlf merge-file -p \
> >>> +		nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >nolf.txt &&
> >>> +	test $(tr "\015" Q <nolf.txt | grep "\\.txtQ$" | wc -l) = 0
> >>> +'
> >>> +
> >>
> >> Minor remark:
> >>
> >> Ramsay suggested a test that doesn't use grep or wc and looks like this:
> >>
> >> test_expect_success 'conflict markers contain CRLF when core.eol=crlf' '
> >>   test_must_fail git -c core.eol=crlf merge-file -p \
> >>     nolf-diff1.txt nolf-orig.txt nolf-diff2.txt >output.txt &&
> >>   tr "\015" Q <output.txt | sed -n "/^[<=>|].*Q$/p" >out.txt &&
> >>   cat >expect.txt <<-\EOF &&
> >>   <<<<<<< nolf-diff1.txtQ
> >>   ||||||| nolf-orig.txtQ
> >>   =======Q
> >>   >>>>>>> nolf-diff2.txtQ
> >>   EOF
> >>   test_cmp expect.txt out.txt
> >> '
> > 
> > Probably he wrapped it at less than 192 columns per row, though ;-)
> > 
> ;-)
> > Seriously again, this longer version might test more, but it definitely
> > also tests more than what I actually want to test: I am simply interested
> > to verify that the conflict markers end in CR/LF when appropriate.
> 
> But you are only testing 3/4 conflict markers end in CR/LF. :-D

The fact that ||| markers are present is the fault of previous test cases.
I tried to make a point of *not* relying on such a side effect (so as to
debug failures quicker by commenting out all previous test cases).

So the fact that I am testing only 3 of the 4 conflict markers is very
much by design.

> > Read: I am uncertain that I want to spend the additional lines on
> > testing more than actually necessary.
> 
> If the here doc is too verbose for you, how about something like this
> (totally untested):
> 
>     test $(tr "\015" Q <crlf.txt | grep "^[<=>|].*Q$" | wc -l) -eq 4
> 
> instead?

Hmm. I do not see the benefit over grepping for `txtQ$` it's essentially
the same.

> HTH

How the hell?

> ATB,
> Ramsay Jones

Authorization to Buy?

Ciao,
Dscho