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

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

Hi Junio,

On Wed, 27 Jan 2016, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> [administrivia: I finally managed to correct e-mail address of peff@,
> which has been giving me bounces for all the previous messages in
> the thread]

Oops! I got those bounces myself, but wrote them off to Eric's account
without checking (because I consistently had problems sending emails to
him in the past). Sorry about that.

> > Your response is also an indicator to me that future myself will find the
> > same code just as confusing as you did, though.
> >
> > Maybe need_cr -> eol_is_crlf?
> 
> The crucial question is how well the variable conveys its meaning
> when it has -1 as its value.  "need_cr? -- I don't know yet" is as
> clear as "eol_is_crlf? -- I don't know yet", if not clearer.  I
> personally do not think "eol_is_crlf" is an improvement.

Okay, then. Let's keep the "needs_cr" name.

Ciao,
Dscho
> it unclear _whose_ eol the variable is talking about.  It can be
> misread as talking about one or more of the files that are being
> merged and mistaken as a statement of a fact (i.e. "we inspected
> and know the input file's eol is CRLF").
> 
> Compared to that, it is clear that "need_cr" is talking about what
> the EOL convention the resulting file should be.
> 
> I briefly wondered if the if/if (need_cr)/... cascade that inspects
> (conditionally up to) three variants might become cleaner if the
> polarity of that variable is flipped (we are allowed to use CRLF
> only when we know that among all of the three variants that we can
> determine the line termination convention used, all of them use
> CRLF), but I didn't think it through seriously.
> 
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>