Subject: Re: [PATCH/RFC v1 1/1] New way to normalize the line endings

From: Junio C Hamano <[email-masked]>
Date: Tue, 29 Nov 2016 11:15:35 -0800

[email-masked] writes:

> From: Torsten Bögershausen <[email-masked]>
>
> Sincec commit 6523728499e7 'convert: unify the "auto" handling of CRLF'
> the normalization instruction in Documentation/gitattributes.txt
> doesn't work any more.

Aside from s/Sincec/Since/, the above made it sound as if the named
commit was a regression that wants to be reverted, at least to my
first reading.  I think you want to be a bit more clear that we
updated the world order and made it a better place with that commit,
and examples in the doc need to be updated.  To convince readers
that, I think you would need to explain things like why the old way
illustrated in the example was bad, and why the new way is better.

> Update the documentation and add a test case.
>
> Reported by Kristian Adrup
> https://github.com/git-for-windows/git/issues/954
>
> Signed-off-by: Torsten Bögershausen <[email-masked]>
> ---
>  Documentation/gitattributes.txt |  7 +++----
>  t/t0025-crlf-auto.sh            | 29 +++++++++++++++++++++++++++++
>  2 files changed, 32 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
> index 976243a..1f7529a 100644
> --- a/Documentation/gitattributes.txt
> +++ b/Documentation/gitattributes.txt
> @@ -227,11 +227,10 @@ From a clean working directory:
>  
>  -------------------------------------------------
>  $ echo "* text=auto" >.gitattributes
> -$ rm .git/index     # Remove the index to force Git to
> -$ git reset         # re-scan the working directory
> +$ git ls-files --eol | egrep "i/(crlf|mixed)" # find not normalized files

Does this step help anything?  I do not see anything in the later
steps that the user uses the finding from the output of this step to
affect the end result.

> +$ rm .git/index     # Remove the index to re-scan the working directory
> +$ git add .

"A clean working directory" usually means all paths in the index
match what's in the working tree but this requires a bit more than
that, as this step ends up adding untracked and unignored paths.

>  $ git status        # Show files that will be normalized
> -$ git add -u
> -$ git add .gitattributes
>  $ git commit -m "Introduce end-of-line normalization"
>  -------------------------------------------------
>  
> diff --git a/t/t0025-crlf-auto.sh b/t/t0025-crlf-auto.sh
> index d0bee08..4ad4d02 100755
> --- a/t/t0025-crlf-auto.sh
> +++ b/t/t0025-crlf-auto.sh
> @@ -152,4 +152,33 @@ test_expect_success 'eol=crlf _does_ normalize binary files' '
>  	test -z "$LFwithNULdiff"
>  '
>  
> +test_expect_success 'prepare unnormalized' '
> +
> +	> .gitattributes &&

Lose SP before ".gitattributes".

> +	git config core.autocrlf false &&
> +	printf "LINEONE\nLINETWO\r\n"     >mixed &&
> +	git add mixed .gitattributes &&
> +	git commit -m "Add mixed" &&
> +	git ls-files --eol | egrep "i/crlf" &&
> +	git ls-files --eol | egrep "i/mixed"
> +
> +'

Any particular reason why we must use egrep not grep here?

> +
> +test_expect_success 'normalize unnormalized' '
> +	echo "* text=auto" >.gitattributes &&
> +	rm .git/index &&
> +	git add . &&
> +	git commit -m "Introduce end-of-line normalization" &&
> +	git ls-files --eol | tr "\\t" " " | sort >act &&
> +cat >exp <<EOF &&
> +i/-text w/-text attr/text=auto         LFwithNUL
> +i/lf    w/crlf  attr/text=auto         CRLFonly
> +i/lf    w/crlf  attr/text=auto         LFonly
> +i/lf    w/lf    attr/text=auto         .gitattributes
> +i/lf    w/mixed attr/text=auto         mixed
> +EOF

Use <<-EOF to indent the above 7 lines?

> +	test_cmp exp act
> +
> +'
> +
>  test_done