Johannes Schindelin [off-list ref] writes:
The crucial idea hit me yesterday when I took a step back: all
we need to do is to ensure that the end-of-line style is matched
when *all* input files are LF-only, or when they are all CR/LF.
In all other cases, we have mixed line endings anyway.
And to do that, it is sufficient to look at *one single line
ending* in the context. Technically, it does not even have to be
the context, but the first line endings of the first file would
be enough, however it is so much more pleasant if the conflict
marker's eol matches the one of the preceding line.
I like that approach. My understanding of the xdiff/xmerge code is
very rusty and I have some time to re-learn it before I can judge
that the change implements that approach correctly, though X-<.
Thanks.
Hi Junio,
On Sun, 24 Jan 2016, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
The crucial idea hit me yesterday when I took a step back: all
we need to do is to ensure that the end-of-line style is matched
when *all* input files are LF-only, or when they are all CR/LF.
In all other cases, we have mixed line endings anyway.
And to do that, it is sufficient to look at *one single line
ending* in the context. Technically, it does not even have to be
the context, but the first line endings of the first file would
be enough, however it is so much more pleasant if the conflict
marker's eol matches the one of the preceding line.
I like that approach. My understanding of the xdiff/xmerge code is
very rusty and I have some time to re-learn it before I can judge
that the change implements that approach correctly, though X-<.
I hear ya. The xmerge code is my fault, too, and it did not help that I
imitated the lack of documentation of the surrounding code.
But it all came back to me when I looked at the code. Hopefully these
hints are helpful:
- xe1 and xe2 refer to "xdfenv_t" types that essentially hold pointers to
one pre-image and one post-image, each
- The pre-images of xe1 and xe2 are identical, of course, because we are
looking at a three-way merge
- The xdfenv_t type refers to the pre-image as xdf1 and to the post-image
as xdf2, both of the type "xdfile_t"
- The xdfile_t structure contains the pointers to a parsed file. The lines
are called "recs" (record) here
- Each record has a pointed to the start of the line and a size that
includes the newline byte, if any
- The record parsing hard-codes 0x0a as newline (NEWLINEBYTES), which is
used in the xdl_hash_record() function that is called from the
xdl_prepare_ctx() function that parses each input file, and which in
turn is called from the xdl_prepare_env() function that prepares a
pair of files for diff'ing by the xdl_do_diff() function that in turn
is called twice from xdl_merge() to diff orig->a and orig->b
- Imitating libdiff's existing code, "i" is used to refer to a specific
line; it is the line number decremented by one, i.e. 0 <= i < nrec
(where "nrec" is the field of the xdfile_t structure indicating the
line count)
You know, cobbling these notes together had an unexpected side effect: I
realized that another patch is needed to properly support CR/LF files:
when the conflict hunk is at the end of one file and said file has no
trailing line ending, we add \n always, but we should add \r before that
when appropriate.
I'm on it.
Ciao,
Dscho