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

[Johannes Schindelin]

Hi Junio,

On Mon, 25 Jan 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > We actually do not have to look at the *entire* context at all: if the
> > files are all LF-only, or if they all have CR/LF line endings, it is
> > sufficient to look at just a *single* line to match that style. And if
> > the line endings are mixed anyway, it is *still* okay to imitate just a
> > single line's eol: we will just add to the pile of mixed line endings,
> > and there is nothing we can do about that.
> 
> Isn't there one thing we can do still?  If we use CRLF for the
> marker lines when the content is already mixed, I'd think it would
> help Notepad (not necessary for Notepad2 or Wordpad IIUC) by making
> sure that they can see where the marker lines end correctly.

Not sure. You might end up with a very long line (containing plenty of LF
"characters") and the conflict marker *at the end* of said line, with a
CR/LF after it. I would not call that particularly helpful.

Seeing as we really cannot do anything in this case, I thought it would be
a good idea to avoid trying (and failing) to be smart here.

> > Note that while it is true that there have to be at least two lines we
> > can look at (otherwise there would be no conflict), the same is not true
> > for line *endings*: the three files in question could all consist of a
> > single line without any line ending, each. In this case we fall back to
> > using LF-only.
> 
> Yeah, this is tricky, and from the same "helping Notepad that
> concatenates lines with LF-only" perspective I should perhaps be
> suggesting to use CRLF in such a case, too, but I would say we
> should not do so.  Three variants of a LF-only file may have
> conflict at the incomplete last line, and if we only look at their
> "no EOL"-ness and decide to add CRLF to the result, that would be
> irritatingly wrong.

Oh, but there is the fall-back to the first line. So if we have three
variants of an LF-only file, the logic will figure out that LF-only
end-of-lines are to be used.

So the *only* case where we really have to pick and choose is when all
three files contain only one (or no) line that is not terminated by a line
feed.

I briefly considered to choose EOL_NATIVE in that case, but I really do
not like that unnecessary deviation from Linux Git.

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

On Tue, Jan 26, 2016 at 1:04 AM, Johannes Schindelin
 wrote:
> Not sure. You might end up with a very long line (containing plenty of LF
> "characters") and the conflict marker *at the end* of said line, with a
> CR/LF after it. I would not call that particularly helpful.
>
> Seeing as we really cannot do anything in this case, I thought it would be
> a good idea to avoid trying (and failing) to be smart here.

Nicely and clearly explained; thanks, and I agree with your reasoning.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Johannes Schindelin  writes:

> We actually do not have to look at the *entire* context at all: if the
> files are all LF-only, or if they all have CR/LF line endings, it is
> sufficient to look at just a *single* line to match that style. And if
> the line endings are mixed anyway, it is *still* okay to imitate just a
> single line's eol: we will just add to the pile of mixed line endings,
> and there is nothing we can do about that.

Isn't there one thing we can do still?  If we use CRLF for the
marker lines when the content is already mixed, I'd think it would
help Notepad (not necessary for Notepad2 or Wordpad IIUC) by making
sure that they can see where the marker lines end correctly.

I do not care too deeply about this; just throwing it out as a
possibility to help Windowsy folks a bit more.

> Note that while it is true that there have to be at least two lines we
> can look at (otherwise there would be no conflict), the same is not true
> for line *endings*: the three files in question could all consist of a
> single line without any line ending, each. In this case we fall back to
> using LF-only.

Yeah, this is tricky, and from the same "helping Notepad that
concatenates lines with LF-only" perspective I should perhaps be
suggesting to use CRLF in such a case, too, but I would say we
should not do so.  Three variants of a LF-only file may have
conflict at the incomplete last line, and if we only look at their
"no EOL"-ness and decide to add CRLF to the result, that would be
irritatingly wrong.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Torsten Bögershausen]

On 25.01.16 09:07, Johannes Schindelin wrote:
[]
> diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
> index 625198e..c852acc 100644
> --- a/xdiff/xmerge.c
> +++ b/xdiff/xmerge.c
> @@ -143,6 +143,35 @@ static int xdl_orig_copy(xdfenv_t *xe, int i, int count, 
> int add_nl, char *dest)
>   return xdl_recs_copy_0(1, xe, i, count, add_nl, dest);
>  }
>  
> +/*
> + * Returns 1 if the i'th line ends in CR/LF (if it is the last line and
> + * has no eol, the preceding line, if any), 0 if it ends in LF-only, and
> + * -1 if the line ending cannot be determined.
> + */
> +static int is_eol_crlf(xdfile_t *file, int i)
Minot nit: Could that be 
is_eol_crlf(xdfile_t *file, int lineno)
(or may be)
is_eol_crlf(const xdfile_t *file, int lineno)
(or may be)
has_eol_crlf(const xdfile_t *file, int lineno)

> +{
> + long size;
> +
> + if (i < file->nrec - 1)
> + /* All lines before the last *must* end in LF */
> + return (size = file->recs[i]->size) > 1 &&
> + file->recs[i]->ptr[size - 2] == '\r';
> + if (!file->nrec)
> + /* Cannot determine eol style from empty file */
> + return -1;
> + if ((size = file->recs[i]->size) &&
> + file->recs[i]->ptr[size - 1] == '\n')
> + /* Last line; ends in LF; Is it CR/LF? */
> + return size > 1 &&
> + file->recs[i]->ptr[size - 2] == '\r';
> + if (!i)
> + /* The only line has no eol */
> + return -1;
> + /* Determine eol from second-to-last line */
> + return (size = file->recs[i - 1]->size) > 1 &&
> + file->recs[i - 1]->ptr[size - 2] == '\r';
> +}
> +

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Schindelin]

Hi Torsten,

On Mon, 25 Jan 2016, Torsten Bögershausen wrote:

> On 25.01.16 09:07, Johannes Schindelin wrote:
> []
> > diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
> > index 625198e..c852acc 100644
> > --- a/xdiff/xmerge.c
> > +++ b/xdiff/xmerge.c
> > @@ -143,6 +143,35 @@ static int xdl_orig_copy(xdfenv_t *xe, int i, int 
> > count, int add_nl, char *dest)
> > return xdl_recs_copy_0(1, xe, i, count, add_nl, dest);
> >  }
> >  
> > +/*
> > + * Returns 1 if the i'th line ends in CR/LF (if it is the last line and
> > + * has no eol, the preceding line, if any), 0 if it ends in LF-only, and
> > + * -1 if the line ending cannot be determined.
> > + */
> > +static int is_eol_crlf(xdfile_t *file, int i)
> Minot nit: Could that be 
> is_eol_crlf(xdfile_t *file, int lineno)
> (or may be)
> is_eol_crlf(const xdfile_t *file, int lineno)
> (or may be)
> has_eol_crlf(const xdfile_t *file, int lineno)

The surrounding code consistently uses i, and not lineno. The reason (I
guess) is that i starts at 0 whereas lineno would have to start at 1, and
we can easily avoid adding and subtracting 1 all the time.

Ciao,
Dscho