Re: [PATCH 2/2] merge-file: consider core.crlf when writing merge markers

[Junio C Hamano <gitster@pobox.com>]

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

>> > It is the duty of ll_merge()'s *caller* (in case of Git's `merge`
>> > command, the merge_content() function) to convert the merge result
>> > into the correct working file contents, and ll_merge() should not muck
>> > with line endings at all.
>> 
>> Hmph, aren't there people who use CRLF throughout their set-up (that is,
>> it is normal for both of their blobs and working tree files to use CRLF
>> line endings)?  Or is such a setting illegal and unsupported?
>
> Good point.
>
> While I would love to simply not support such a case, it would be turning
> a blind eye to reality.
>
> So I guess I need another patch like this (plus a test case):
>
> -- snipsnap --
> t a/ll-merge.c b/ll-merge.c
> index 0338630..4a565c8 100644
> --- a/ll-merge.c
> +++ b/ll-merge.c
> @@ -111,6 +111,7 @@ static int ll_xdl_merge(const struct ll_merge_driver
> *drv_unused,
>  		xmp.style = git_xmerge_style;
>  	if (marker_size > 0)
>  		xmp.marker_size = marker_size;
> +	xmp.crlf = eol_for_path(NULL, src1->ptr, src1->size) == EOL_CRLF;
>  	xmp.ancestor = orig_name;
>  	xmp.file1 = name1;
>  	xmp.file2 = name2;

What I do not understand is that you already had xmp.crlf even the
log message claimed that the caller is supposed to feed LF blobs and
the ll_merge() shouldn't have to worry about this.

If the user sets the repository up to use CRLF in working tree and
LF in blob (e.g. crlf = input), shouldn't cmd_merge_file() be doing
the convert_to_git() to the buffer stored in mmfs[] before calling
down to xdl_merge() so that ll_merge() layer will see LF not CRLF?

And if the interface is truly done in such a way that was outlined
in the proposed log message, I do not think xmp.crlf is a good name
for the new field.  What the updated code needs is a boolean option,
xmp.end_marker_with_crlf, that is set only when the internal blob
encoding is CRLF and affects only the ending of the marker strings.

It looks to me that the code is doing something different from what
the proposed log message claimed, which is puzzling.