Consequences of CRLF in index?

> However, I wonder if there are any "hidden consequences" of this setup?
> If there are consequences, then I see two options. Either I rebase the repo
> and fix the line endings for all commits or I add a single commit that fixes
> the line endings for all files. Both actions require coordination with the
> users to avoid repo trouble/merge conflicts. The "single fixup commit" options
> would also make diffs into the past look bad. Would a single large commit have
> any impact on the performance of standard Git operations?

> Hi,
> 
> Lars Schneider wrote:
> 
> > I've migrated a large repo (110k+ files) with a lot of history (177k commits)
> > and a lot of users (200+) to Git. Unfortunately, all text files in the index
> > of the repo have CRLF line endings. In general this seems not to be a problem
> > as the project is developed exclusively on Windows.
> 
> Sounds good.
> 
> > However, I wonder if there are any "hidden consequences" of this setup?
> > If there are consequences, then I see two options. Either I rebase the repo
> > and fix the line endings for all commits or I add a single commit that fixes
> > the line endings for all files. Both actions require coordination with the
> > users to avoid repo trouble/merge conflicts. The "single fixup commit" options
> > would also make diffs into the past look bad. Would a single large commit have
> > any impact on the performance of standard Git operations?
> 
> There are no hidden consequences that I'm aware of.  If you later
> decide that you want to become a cross-platform project, then you may
> want to switch to LF endings, in which case I suggest the "single
> fixup commit" strategy.
> 
> In any event, you also probably want to declare what you're doing
> using .gitattributes.  By checking in the files as CRLF, you are
> declaring that you do *not* want Git to treat them as text files
> (i.e., you do not want Git to change the line endings), so something
> as simple as
> 
> 	* -text
> 
> should do the trick.  See gitattributes(5) for details.

> 
> I'd be interested to hear what happens when diff-ing across a line
> ending fixup commit.  Is this an area where Git needs some
> improvement?  "git merge" knows an -Xrenormalize option to deal with a
> related problem --- it's possible that "git diff" needs to learn a
> similar trick.

> I'd be interested to hear what happens when diff-ing across a line
> ending fixup commit.  Is this an area where Git needs some
> improvement?  "git merge" knows an -Xrenormalize option to deal with a
> related problem --- it's possible that "git diff" needs to learn a
> similar trick.

> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> I'd be interested to hear what happens when diff-ing across a line
>> ending fixup commit.  Is this an area where Git needs some
>> improvement?  "git merge" knows an -Xrenormalize option to deal with a
>> related problem --- it's possible that "git diff" needs to learn a
>> similar trick.
>
> My knee-jerk reaction is that (1) the end user definitely wants to
> see preimage and postimage lines are different in such a commit by
> default, one side has and the other side lacks ^M at the end., and
> (2) one of the "whitespace ignoring" options (namely, "ignore space
> at eol") may suffice, but if not, it should be easy to invent a new
> one "ignore CR at eol".

>
> Here is a lunch-time hack to add that option.  As you can see from
> the lack of docs, tests and a proper log message, I haven't played
> with it long enough to know how buggy it is, though.  I am not all
> that interested in polishing it to completion myself and prefer to
> leave it as #leftoverbits ;-)

>  diff.c            |  5 ++++-
>  merge-recursive.c |  2 ++
>  xdiff/xdiff.h     |  3 ++-
>  xdiff/xutils.c    | 34 ++++++++++++++++++++++++++++++++--
>  4 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 6fd288420b..eeca0fd3b8 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4202,7 +4202,8 @@ void diff_setup_done(struct diff_options *options)
>
>         if (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||
>             DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||
> -           DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL))
> +           DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL) ||
> +           DIFF_XDL_TST(options, IGNORE_CR_AT_EOL))

>  #define XDF_IGNORE_WHITESPACE_AT_EOL (1 << 4)
> -#define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE | XDF_IGNORE_WHITESPACE_AT_EOL)
> +#define XDF_IGNORE_CR_AT_EOL (1 << 5)
> +#define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE | XDF_IGNORE_WHITESPACE_AT_EOL | XDF_IGNORE_CR_AT_EOL)
>
>  #define XDF_PATIENCE_DIFF (1 << 5)

>  #define XDF_HISTOGRAM_DIFF (1 << 6)
> diff --git a/xdiff/xutils.c b/xdiff/xutils.c
> index 04d7b32e4e..8720e272b9 100644
> --- a/xdiff/xutils.c
> +++ b/xdiff/xutils.c
> @@ -156,6 +156,21 @@ int xdl_blankline(const char *line, long size, long flags)
>         return (i == size);
>  }
>
> +/*
> + * Have we eaten everything on the line, except for an optional
> + * CR at the very end?
> + */
> +static int ends_with_optional_cr(const char *l, long s, long i)
> +{
> +       if (s && l[s-1] == '\n')
> +               s--;

>          * Each flavor of ignoring needs different logic to skip whitespaces
>          * while we have both sides to compare.
> @@ -204,6 +220,14 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)
>                         i1++;
>                         i2++;
>                 }
> +       } else if (flags & XDF_IGNORE_CR_AT_EOL) {
> +               /* Find the first difference and see how the line ends */
> +               while (i1 < s1 && i2 < s2 && l1[i1] == l2[i2]) {
> +                       i1++;
> +                       i2++;
> +               }
> +               return (ends_with_optional_cr(l1, s1, i1) &&
> +                       ends_with_optional_cr(l2, s2, i2));
>         }
>
>         /*
> @@ -230,9 +254,15 @@ static unsigned long xdl_hash_record_with_whitespace(char const **data,
>                 char const *top, long flags) {
>         unsigned long ha = 5381;
>         char const *ptr = *data;
> +       int cr_at_eol_only = (flags & XDF_WHITESPACE_FLAGS) == XDF_IGNORE_CR_AT_EOL;
>
>         for (; ptr < top && *ptr != '\n'; ptr++) {
> -               if (XDL_ISSPACE(*ptr)) {
> +               if (cr_at_eol_only) {
> +                       if (*ptr == '\r' &&
> +                           (top <= ptr + 1 || ptr[1] == '\n'))
> +                               continue;
> +               }
> +               else if (XDL_ISSPACE(*ptr)) {
>                         const char *ptr2 = ptr;
>                         int at_eol;
>                         while (ptr + 1 < top && XDL_ISSPACE(ptr[1])
>
>

>> diff --git a/diff.c b/diff.c
>> index 6fd288420b..eeca0fd3b8 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -4202,7 +4202,8 @@ void diff_setup_done(struct diff_options *options)
>>
>>         if (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||
>>             DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||
>> -           DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL))
>> +           DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL) ||
>> +           DIFF_XDL_TST(options, IGNORE_CR_AT_EOL))
>
> This highlights another part of the flag macros, that could be made nicer.
> All these flags combined are XDF_WHITESPACE_FLAGS, so this
> if could be written without the macros as
>
>   if (options->xdl_ops & XDF_WHITESPACE_FLAGS)

>> diff --git a/xdiff/xutils.c b/xdiff/xutils.c
>> index 04d7b32e4e..8720e272b9 100644
>> --- a/xdiff/xutils.c
>> +++ b/xdiff/xutils.c
>> @@ -156,6 +156,21 @@ int xdl_blankline(const char *line, long size, long flags)
>>         return (i == size);
>>  }
>>
>> +/*
>> + * Have we eaten everything on the line, except for an optional
>> + * CR at the very end?
>> + */
>> +static int ends_with_optional_cr(const char *l, long s, long i)
>> +{
>> +       if (s && l[s-1] == '\n')
>> +               s--;
>
> so first we cut off the '\n',

> I might offer
> another easier to understand (for me) solution,
> ...
> Though this seems even more complicated
> after having it written down.

> Stefan Beller <sbeller@google.com> writes:
>
>> (1<<5) is taken twice now.
>
> Good eyes.  I think we use bits #1-#8 now (bit #0 is vacant, so are
> #9-#31).

> Subject: [PATCH] xdiff: reassign xpparm_t.flags bits
>
> We have packed the bits too tightly in such a way that it is not
> easy to add a new type of whitespace ignoring option, a new type
> of LCS algorithm, or a new type of post-cleanup heuristics.
>
> Reorder bits a bit to give room for these three classes of options
> to grow.
>
> While at it, add a comment in front of the bit definitions to
> clarify in which structure these defined bits may appear.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

> ---
>  xdiff/xdiff.h | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
> index b090ad8eac..457cac32d8 100644
> --- a/xdiff/xdiff.h
> +++ b/xdiff/xdiff.h
> @@ -27,22 +27,24 @@
>  extern "C" {
>  #endif /* #ifdef __cplusplus */
>
> +/* xpparm_t.flags */
> +#define XDF_NEED_MINIMAL (1 << 0)
>
> -#define XDF_NEED_MINIMAL (1 << 1)

>  #define XDF_IGNORE_WHITESPACE (1 << 2)
>  #define XDF_IGNORE_WHITESPACE_CHANGE (1 << 3)
>  #define XDF_IGNORE_WHITESPACE_AT_EOL (1 << 4)
>  #define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE | XDF_IGNORE_WHITESPACE_AT_EOL)
>
> -#define XDF_PATIENCE_DIFF (1 << 5)
> -#define XDF_HISTOGRAM_DIFF (1 << 6)
> +#define XDF_IGNORE_BLANK_LINES (1 << 7)

> +#define XDF_PATIENCE_DIFF (1 << 14)
> +#define XDF_HISTOGRAM_DIFF (1 << 15)
>  #define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF)
>  #define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK)
>
> -#define XDF_IGNORE_BLANK_LINES (1 << 7)
> -
> -#define XDF_INDENT_HEURISTIC (1 << 8)
> +#define XDF_INDENT_HEURISTIC (1 << 23)
>
> +/* xdemitconf_t.flags */
>  #define XDL_EMIT_FUNCNAMES (1 << 0)
>  #define XDL_EMIT_FUNCCONTEXT (1 << 2)

> Stefan Beller <sbeller@google.com> writes:
>
>> (I note this as you regard your patches as a lunch time hack
>> in the cooking email; I am serious about these patches though.)
>
> We do not want to touch borrowed code unnecessarily.  Are these
> lines and bits hampering further progress we are actively trying to
> make right now?

> On Mon, Oct 30, 2017 at 7:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Stefan Beller <sbeller@google.com> writes:
> >
> >> (I note this as you regard your patches as a lunch time hack
> >> in the cooking email; I am serious about these patches though.)
> >
> > We do not want to touch borrowed code unnecessarily.  Are these
> > lines and bits hampering further progress we are actively trying to
> > make right now?
> 
> No they are not, you are correct.
> 
> I differ in opinion on 'borrowed code'. The latest release of xdiff
> (v0.23) was in Nov 13, 2008 according to http://freecode.com/projects/xdiff-lib
> or on March 23, 2006 according to https://directory.fsf.org/wiki/LibXDiff
> and given that we incorporated so many changes already to xdiff,
> I would argue it is sufficiently different from the original, we'll probably
> never import another upstream version (if there will be a release at all).
> 
> So the code was rather taken and now we are the bag holders in
> maintaining it, so we can make it pretty even only for the sake of
> pleasing ourselves (or rather: not confusing ourselves with too
> many unused flags).

> A new option --ignore-cr-at-eol tells the diff machinery to treat a
> carriage-return at the end of a (complete) line as if it does not
> exist.
> 
> This would make it easier to review a change whose only effect is to
> turn line endings from CRLF to LF or the other way around.

> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 89cc0f48de..aa2c0ff74d 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -519,6 +519,9 @@ endif::git-format-patch[]
>  --text::
>  	Treat all files as text.
>  
> +--ignore-cr-at-eol::
> +	Ignore carrige-return at the end of line when doing a comparison.

> diff --git a/xdiff/xutils.c b/xdiff/xutils.c
> index 04d7b32e4e..b2cbcc818f 100644
> --- a/xdiff/xutils.c
> +++ b/xdiff/xutils.c
> @@ -156,6 +156,24 @@ int xdl_blankline(const char *line, long size, long flags)
>  	return (i == size);
>  }
>  
> +/*
> + * Have we eaten everything on the line, except for an optional
> + * CR at the very end?
> + */
> +static int ends_with_optional_cr(const char *l, long s, long i)
> +{
> +	int complete = s && l[s-1] == '\n';
> +
> +	if (complete)
> +		s--;
> +	if (s == i)
> +		return 1;

> @@ -204,6 +223,14 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)
>  			i1++;
>  			i2++;
>  		}
> +	} else if (flags & XDF_IGNORE_CR_AT_EOL) {
> +		/* Find the first difference and see how the line ends */
> +		while (i1 < s1 && i2 < s2 && l1[i1] == l2[i2]) {
> +			i1++;
> +			i2++;
> +		}
> +		return (ends_with_optional_cr(l1, s1, i1) &&
> +			ends_with_optional_cr(l2, s2, i2));

> On Tue, 7 Nov 2017, Junio C Hamano wrote:
>
>> A new option --ignore-cr-at-eol tells the diff machinery to treat a
>> carriage-return at the end of a (complete) line as if it does not
>> exist.
>> 
>> This would make it easier to review a change whose only effect is to
>> turn line endings from CRLF to LF or the other way around.
>
> If the goal is to make CR/LF -> LF conversions easier to review (or for
> that matter, LF -> CR/LF), then this option may not be *completely*
> satisfactory, as it would hide mixed changes (i.e. where some lines are
> converted from CR/LF to LF and others are converted in the other direction
> *in the same patch*).

>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> index 89cc0f48de..aa2c0ff74d 100644
>> --- a/Documentation/diff-options.txt
>> +++ b/Documentation/diff-options.txt
>> @@ -519,6 +519,9 @@ endif::git-format-patch[]
>>  --text::
>>  	Treat all files as text.
>>  
>> +--ignore-cr-at-eol::
>> +	Ignore carrige-return at the end of line when doing a comparison.
>
> I am not a native speaker, either, yet I have the impression that "do a
> comparison" may be more colloquial than not. Also, it is a carriage-return
> (as in Sinatra's famous song about Love and Marriage) not a carrige-return.
>
> How about "Hide changed line endings"?

>> diff --git a/xdiff/xutils.c b/xdiff/xutils.c
>> index 04d7b32e4e..b2cbcc818f 100644
>> --- a/xdiff/xutils.c
>> +++ b/xdiff/xutils.c
>> @@ -156,6 +156,24 @@ int xdl_blankline(const char *line, long size, long flags)
>>  	return (i == size);
>>  }
>>  
>> +/*
>> + * Have we eaten everything on the line, except for an optional
>> + * CR at the very end?
>> + */
>> +static int ends_with_optional_cr(const char *l, long s, long i)
>> +{
>> +	int complete = s && l[s-1] == '\n';
>> +
>> +	if (complete)
>> +		s--;
>> +	if (s == i)
>> +		return 1;
>
> What is the role of `s`, what of `i`? Maybe `length` and `current_offset`?

>> +	/* do not ignore CR at the end of an incomplete line */
>> +	if (complete && s == i + 1 && l[i] == '\r')
>> +		return 1;
>
> This made me scratch my head: too many negations. The comment may better
> read "ignore CR only at the end of a complete line".

>> @@ -204,6 +223,14 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)
>>  			i1++;
>>  			i2++;
>>  		}
>> +	} else if (flags & XDF_IGNORE_CR_AT_EOL) {
>> +		/* Find the first difference and see how the line ends */
>> +		while (i1 < s1 && i2 < s2 && l1[i1] == l2[i2]) {
>> +			i1++;
>> +			i2++;
>> +		}
>> +		return (ends_with_optional_cr(l1, s1, i1) &&
>> +			ends_with_optional_cr(l2, s2, i2));
>
> There are extra parentheses around the `return` expression.

> We have packed the bits too tightly in such a way that it is not
> easy to add a new type of whitespace ignoring option, a new type
> of LCS algorithm, or a new type of post-cleanup heuristics.
> 
> Reorder bits a bit to give room for these three classes of options
> to grow.  Also make use of XDF_WHITESPACE_FLAGS macro where we check
> any of these bits are on, instead of using DIFF_XDL_TST() macro on
> individual possibilities.  That way, the "is any of the bits on?"
> code does not have to change when we add more ways to ignore
> whitespaces.
> 
> While at it, add a comment in front of the bit definitions to
> clarify in which structure these defined bits may appear.

> diff --git a/diff.c b/diff.c
> index 74283d9001..790250fe86 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3434,9 +3434,7 @@ void diff_setup_done(struct diff_options *options)
>  	 * inside contents.
>  	 */
>  
> -	if (DIFF_XDL_TST(options, IGNORE_WHITESPACE) ||
> -	    DIFF_XDL_TST(options, IGNORE_WHITESPACE_CHANGE) ||
> -	    DIFF_XDL_TST(options, IGNORE_WHITESPACE_AT_EOL))
> +	if ((options->xdl_opts & XDF_WHITESPACE_FLAGS))

>  		DIFF_OPT_SET(options, DIFF_FROM_CONTENTS);
>  	else
>  		DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);
> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
> index b090ad8eac..cbf5d8e166 100644
> --- a/xdiff/xdiff.h
> +++ b/xdiff/xdiff.h
> @@ -27,22 +27,26 @@
>  extern "C" {
>  #endif /* #ifdef __cplusplus */
>  
> +/* xpparm_t.flags */
> +#define XDF_NEED_MINIMAL (1 << 0)
>  
> -#define XDF_NEED_MINIMAL (1 << 1)

> -#define XDF_IGNORE_WHITESPACE (1 << 2)
> -#define XDF_IGNORE_WHITESPACE_CHANGE (1 << 3)
> -#define XDF_IGNORE_WHITESPACE_AT_EOL (1 << 4)
> -#define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE | XDF_IGNORE_WHITESPACE_AT_EOL)
> +#define XDF_IGNORE_WHITESPACE (1 << 1)
> +#define XDF_IGNORE_WHITESPACE_CHANGE (1 << 2)
> +#define XDF_IGNORE_WHITESPACE_AT_EOL (1 << 3)
> +#define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | \
> +			      XDF_IGNORE_WHITESPACE_CHANGE | \
> +			      XDF_IGNORE_WHITESPACE_AT_EOL)
>  
> -#define XDF_PATIENCE_DIFF (1 << 5)
> -#define XDF_HISTOGRAM_DIFF (1 << 6)
> +#define XDF_IGNORE_BLANK_LINES (1 << 7)
> +
> +#define XDF_PATIENCE_DIFF (1 << 14)
> +#define XDF_HISTOGRAM_DIFF (1 << 15)
>  #define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF)
>  #define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK)
>  
> -#define XDF_IGNORE_BLANK_LINES (1 << 7)
> -
> -#define XDF_INDENT_HEURISTIC (1 << 8)
> +#define XDF_INDENT_HEURISTIC (1 << 23)
>  
> +/* xdemitconf_t.flags */
>  #define XDL_EMIT_FUNCNAMES (1 << 0)
>  #define XDL_EMIT_FUNCCONTEXT (1 << 2)

>> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
>> index b090ad8eac..cbf5d8e166 100644
>> --- a/xdiff/xdiff.h
>> +++ b/xdiff/xdiff.h
>> @@ -27,22 +27,26 @@
>>  extern "C" {
>>  #endif /* #ifdef __cplusplus */
>>  
>> +/* xpparm_t.flags */
>> +#define XDF_NEED_MINIMAL (1 << 0)
>>  
>> -#define XDF_NEED_MINIMAL (1 << 1)
>
> This change makes me wonder whether the least significant bit was omitted
> on purpose originally. You probably looked at that? May be worth
> mentioning in the commit message.
>
>> -#define XDF_IGNORE_WHITESPACE (1 << 2)
>> -#define XDF_IGNORE_WHITESPACE_CHANGE (1 << 3)
>> -#define XDF_IGNORE_WHITESPACE_AT_EOL (1 << 4)
>> -#define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE | XDF_IGNORE_WHITESPACE_AT_EOL)
>> +#define XDF_IGNORE_WHITESPACE (1 << 1)
>> +#define XDF_IGNORE_WHITESPACE_CHANGE (1 << 2)
>> +#define XDF_IGNORE_WHITESPACE_AT_EOL (1 << 3)
>> +#define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | \
>> +			      XDF_IGNORE_WHITESPACE_CHANGE | \
>> +			      XDF_IGNORE_WHITESPACE_AT_EOL)
>>  
>> -#define XDF_PATIENCE_DIFF (1 << 5)
>> -#define XDF_HISTOGRAM_DIFF (1 << 6)
>> +#define XDF_IGNORE_BLANK_LINES (1 << 7)
>> +
>> +#define XDF_PATIENCE_DIFF (1 << 14)
>> +#define XDF_HISTOGRAM_DIFF (1 << 15)
>>  #define XDF_DIFF_ALGORITHM_MASK (XDF_PATIENCE_DIFF | XDF_HISTOGRAM_DIFF)
>>  #define XDF_DIFF_ALG(x) ((x) & XDF_DIFF_ALGORITHM_MASK)
>>  
>> -#define XDF_IGNORE_BLANK_LINES (1 << 7)
>> -
>> -#define XDF_INDENT_HEURISTIC (1 << 8)
>> +#define XDF_INDENT_HEURISTIC (1 << 23)
>>  
>> +/* xdemitconf_t.flags */
>>  #define XDL_EMIT_FUNCNAMES (1 << 0)
>>  #define XDL_EMIT_FUNCCONTEXT (1 << 2)
>
> It is a pity that this diff is not easier to review, but it shows how much
> it was in need of cleaning up. Looks much nicer now.
>
> I wonder, however, what your guiding principle was in determining the
> gaps? I would have expected consecutive bits, except for the one gap to
> make room for the upcoming flag, of course.

> This time with doc updates and tests.  The previous one is at 
> 
> https://public-inbox.org/git/xmqqshe7j0af.fsf@gitster.mtv.corp.google.com
> 
> A change that is supposed to only change the end-of-line convention
> between LF <-> CRLF and nothing else can be verified with "diff -b"
> or "diff --ignore-space-at-eol" in practice, but these hide changes
> in whitespaces other than the carriage return at the end of the lines.

> Good. I was wishing for such a feature in the past.
>
> However, the short and sweet `-b` or `-w` switches are really, really
> nice. `--ignore-cr-at-eol` is just very cumbersome to type out. So I think
> you will want to add this patch to your patch series:

> -- snipsnap --
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 0e16f017a41..b7a45e8df29 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1400,7 +1400,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
>  			--patch-with-stat --name-only --name-status --color
>  			--no-color --color-words --no-renames --check
>  			--full-index --binary --abbrev --diff-filter=
> -			--find-copies-harder
> +			--find-copies-harder --ignore-cr-at-eol
>  			--text --ignore-space-at-eol --ignore-space-change
>  			--ignore-all-space --ignore-blank-lines --exit-code
>  			--quiet --ext-diff --no-ext-diff

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> Good. I was wishing for such a feature in the past.
>>
>> However, the short and sweet `-b` or `-w` switches are really, really
>> nice. `--ignore-cr-at-eol` is just very cumbersome to type out. So I think
>> you will want to add this patch to your patch series:
>
> Yeah, I should probably have added that to 2/2 from the beginning.
>
>> -- snipsnap --
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 0e16f017a41..b7a45e8df29 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -1400,7 +1400,7 @@ __git_diff_common_options="--stat --numstat --shortstat --summary
>>                       --patch-with-stat --name-only --name-status --color
>>                       --no-color --color-words --no-renames --check
>>                       --full-index --binary --abbrev --diff-filter=
>> -                     --find-copies-harder
>> +                     --find-copies-harder --ignore-cr-at-eol
>>                       --text --ignore-space-at-eol --ignore-space-change
>>                       --ignore-all-space --ignore-blank-lines --exit-code
>>                       --quiet --ext-diff --no-ext-diff

> On 24 Oct 2017, at 20:14, Jonathan Nieder <jrnieder@gmail.com> wrote:
> 
> Hi,
> 
> Lars Schneider wrote:
> 
>> I've migrated a large repo (110k+ files) with a lot of history (177k commits)
>> and a lot of users (200+) to Git. Unfortunately, all text files in the index
>> of the repo have CRLF line endings. In general this seems not to be a problem
>> as the project is developed exclusively on Windows.
> 
> Sounds good.
> 
>> However, I wonder if there are any "hidden consequences" of this setup?
>> If there are consequences, then I see two options. Either I rebase the repo
>> and fix the line endings for all commits or I add a single commit that fixes
>> the line endings for all files. Both actions require coordination with the
>> users to avoid repo trouble/merge conflicts. The "single fixup commit" options
>> would also make diffs into the past look bad. Would a single large commit have
>> any impact on the performance of standard Git operations?
> 
> There are no hidden consequences that I'm aware of.  If you later
> decide that you want to become a cross-platform project, then you may
> want to switch to LF endings, in which case I suggest the "single
> fixup commit" strategy.
> 
> In any event, you also probably want to declare what you're doing
> using .gitattributes.  By checking in the files as CRLF, you are
> declaring that you do *not* want Git to treat them as text files
> (i.e., you do not want Git to change the line endings), so something
> as simple as
> 
> 	* -text

>> In any event, you also probably want to declare what you're doing
>> using .gitattributes.  By checking in the files as CRLF, you are
>> declaring that you do *not* want Git to treat them as text files
>> (i.e., you do not want Git to change the line endings), so something
>> as simple as
>>
>> 	* -text
>
> That's sounds good. Does "-text" have any other implications?
> For whatever reason I always thought this is the way to tell
> Git that a particular file is binary with the implication that
> Git should not attempt to diff it.

> On 25 Oct 2017, at 19:13, Jonathan Nieder <jrnieder@gmail.com> wrote:
> 
> Hi again,
> 
> Lars Schneider wrote:
>>> On 24 Oct 2017, at 20:14, Jonathan Nieder <jrnieder@gmail.com> wrote:
> 
>>> In any event, you also probably want to declare what you're doing
>>> using .gitattributes.  By checking in the files as CRLF, you are
>>> declaring that you do *not* want Git to treat them as text files
>>> (i.e., you do not want Git to change the line endings), so something
>>> as simple as
>>> 
>>> 	* -text
>> 
>> That's sounds good. Does "-text" have any other implications?
>> For whatever reason I always thought this is the way to tell
>> Git that a particular file is binary with the implication that
>> Git should not attempt to diff it.
> 
> No other implications.  You're thinking of "-diff".  There is also a
> shortcut "binary" which simply means "-text -diff".

> Hi again,
> 
> Lars Schneider wrote:
> >> On 24 Oct 2017, at 20:14, Jonathan Nieder <jrnieder@gmail.com> wrote:
> 
> >> In any event, you also probably want to declare what you're doing
> >> using .gitattributes.  By checking in the files as CRLF, you are
> >> declaring that you do *not* want Git to treat them as text files
> >> (i.e., you do not want Git to change the line endings), so something
> >> as simple as
> >>
> >> 	* -text
> >
> > That's sounds good. Does "-text" have any other implications?
> > For whatever reason I always thought this is the way to tell
> > Git that a particular file is binary with the implication that
> > Git should not attempt to diff it.
> 
> No other implications.  You're thinking of "-diff".  There is also a
> shortcut "binary" which simply means "-text -diff".

> I've migrated a large repo (110k+ files) with a lot of history (177k commits)
> and a lot of users (200+) to Git. Unfortunately, all text files in the index
> of the repo have CRLF line endings. In general this seems not to be a problem
> as the project is developed exclusively on Windows.
> 
> However, I wonder if there are any "hidden consequences" of this setup?

> Am 24.10.2017 um 19:48 schrieb Lars Schneider:
> > I've migrated a large repo (110k+ files) with a lot of history (177k
> > commits)
> > and a lot of users (200+) to Git. Unfortunately, all text files in the index
> > of the repo have CRLF line endings. In general this seems not to be a
> > problem
> > as the project is developed exclusively on Windows.
> > 
> > However, I wonder if there are any "hidden consequences" of this setup?
> 
> I've been working on a project with CRLF in every source file for a decade
> now. It's C++ source, and it isn't even Windows-only: when checked out on
> Linux, there are CRs in the files, with no bad consequences so far. GCC is
> happy with them.

> I envy you for the blessing of such a clean C++ source that you do not
> have any, say, Unix shell script in it. Try this, and weep:
> 
> 	$ printf 'echo \\\r\n\t123\r\n' >a1
> 
> 	$ sh a1
> 
> 	a1: 2: a1: 123: not found

> On 26 Oct 2017, at 09:09, Johannes Sixt <j6t@kdbg.org> wrote:
> 
> Am 25.10.2017 um 14:19 schrieb Johannes Schindelin:
>> I envy you for the blessing of such a clean C++ source that you do not
>> have any, say, Unix shell script in it. Try this, and weep:
>> 	$ printf 'echo \\\r\n\t123\r\n' >a1
>> 	$ sh a1
>> 	a1: 2: a1: 123: not found
> 
> I was bitten by that, too. For this reason, I ensure that shell scripts and Makefiles begin their life on Linux. Fortunately, modern editors on Windows, includ^Wand vi, do not force CRLF line breaks, and such files can be edited on Windows, too.

> 
> > On 26 Oct 2017, at 09:09, Johannes Sixt <j6t@kdbg.org> wrote:
> > 
> > Am 25.10.2017 um 14:19 schrieb Johannes Schindelin:
> >> I envy you for the blessing of such a clean C++ source that you do not
> >> have any, say, Unix shell script in it. Try this, and weep:
> >> 	$ printf 'echo \\\r\n\t123\r\n' >a1
> >> 	$ sh a1
> >> 	a1: 2: a1: 123: not found
> > 
> > I was bitten by that, too. For this reason, I ensure that shell scripts and Makefiles begin their life on Linux. Fortunately, modern editors on Windows, includ^Wand vi, do not force CRLF line breaks, and such files can be edited on Windows, too.
> 
> Wouldn't this kind of .gitattributes setup solve the problem?
> 
> *     -text
> *.sh   text eol=lf
> 

>> On 26 Oct 2017, at 09:09, Johannes Sixt <j6t@kdbg.org> wrote:
>> Am 25.10.2017 um 14:19 schrieb Johannes Schindelin:
>>> I envy you for the blessing of such a clean C++ source that you do not
>>> have any, say, Unix shell script in it. Try this, and weep:
>>> 	$ printf 'echo \\\r\n\t123\r\n' >a1
>>> 	$ sh a1
>>> 	a1: 2: a1: 123: not found
>>
>> I was bitten by that, too. For this reason, I ensure that shell scripts and Makefiles begin their life on Linux. Fortunately, modern editors on Windows, includ^Wand vi, do not force CRLF line breaks, and such files can be edited on Windows, too.
> 
> Wouldn't this kind of .gitattributes setup solve the problem?

>> *     -text
>> *.sh   text eol=lf
>
> Why would that be necessary? I cannot have CRLF in shell scripts etc., not
> even on Windows. (And in addition I do not mind CR in C++ source code.) All
> I want is: Git, please, by all means, do not mess with my files, ever. Isn't
> the simplest way to achieve that to set *nothing* at all? Not even
> core.autocrlf?

> Is "* -text" in any way different than "-text" (without the * asterisk)? All
> of my .gitattributes files have "-text" (no * asterisk) and I haven't noticed
> any difference but could I be missing something subtle?
>
> ~Ross

> > On 26 Oct 2017, at 09:09, Johannes Sixt <j6t@kdbg.org> wrote:
> > 
> > Am 25.10.2017 um 14:19 schrieb Johannes Schindelin:
> >> I envy you for the blessing of such a clean C++ source that you do not
> >> have any, say, Unix shell script in it. Try this, and weep:
> >> 	$ printf 'echo \\\r\n\t123\r\n' >a1
> >> 	$ sh a1
> >> 	a1: 2: a1: 123: not found
> > 
> > I was bitten by that, too. For this reason, I ensure that shell
> > scripts and Makefiles begin their life on Linux. Fortunately, modern
> > editors on Windows, includ^Wand vi, do not force CRLF line breaks, and

> > such files can be edited on Windows, too.
> 
> Wouldn't this kind of .gitattributes setup solve the problem?
> 
> *     -text
> *.sh   text eol=lf