Hi Junio, On Wed, 27 Jan 2016, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html