Re: [PATCH v2 0/1] Let merge-file write out conflict markers with correct EOLs

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

Hi Junio,

On Sun, 24 Jan 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> 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-<.

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