Re: [PATCH] pre-commit hook should ignore carriage returns at EOL

[Ian Hilt <Ian.Hilt@xxxxxxx>]

On Tue, 24 Jun 2008 at 12:23pm -0400, Christian Holtje wrote:

> When commit files that use DOS style CRLF end-of-lines, the pre-commit
> hook would raise an error.  When combined with the fact that the hooks
> get activated by default on windows, it makes life difficult for
> people working with DOS files.
> 
> This patch causes the pre-commit hook to deal with crlf files
> correctly.
> 
> Signed-off-by: Christian Höltje <docwhat@xxxxxxxxx>
> ---
> t/t7503-template-hook--pre-commit.sh |   75 ++++++++++++++++++++++++++++++++++
> templates/hooks--pre-commit          |   10 ++++-
> 2 files changed, 83 insertions(+), 2 deletions(-)
> create mode 100755 t/t7503-template-hook--pre-commit.sh
> 
> diff --git a/t/t7503-template-hook--pre-commit.sh
> b/t/t7503-template-hook--pre-commit.sh
> new file mode 100755
> index 0000000..8f0c3c9
> --- /dev/null
> +++ b/t/t7503-template-hook--pre-commit.sh
> @@ -0,0 +1,75 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2008 Christian Höltje
> +#
> +
> +test_description='t7503 templates-hooks--pre-commit
> +
> +This test verifies that the pre-commit hook shipped with
> +git by default works correctly.
> +'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'verify that autocrlf is unset' '
> +   if git config core.autocrlf
> +   then
> +     false
> +   else
> +     test $? -eq 1
> +   fi
> +'
> +
> +test_expect_success 'lf without hook' '
> +
> +	echo "foo" > lf.txt &&
> +	git add lf.txt &&
> +	git commit -m "lf without hook" lf.txt
> +
> +'
> +
> +test_expect_success 'crlf without hook' '
> +
> +	echo "foo\r" > crlf.txt &&

Perhaps you want to use printf "foo\r" instead?  echo "foo\r" does not
produce a CR on my system.

> +	git add crlf.txt &&
> +	git commit -m "crlf without hook" crlf.txt
> +
> +'
> +
> +# Set up the pre-commit hook.
> +HOOKDIR="$(git rev-parse --git-dir)/hooks"
> +mkdir -p "${HOOKDIR}"
> +cp -r "${HOOKDIR}-disabled/pre-commit" "${HOOKDIR}/pre-commit"
> +chmod +x "${HOOKDIR}/pre-commit"
> +
> +test_expect_success 'lf with hook' '
> +
> +	echo "bar" >> lf.txt &&
> +	git add lf.txt &&
> +	git commit -m "lf with hook" lf.txt
> +
> +'
> +test_expect_success 'crlf with hook' '
> +
> +	echo "bar\r" >> crlf.txt &&
> +	git add crlf.txt &&
> +	git commit -m "crlf with hook" crlf.txt
> +
> +'
> +
> +test_expect_success 'lf with hook white-space failure' '
> +
> +	echo "bar " >> lf.txt &&
> +	git add lf.txt &&
> +	! git commit -m "lf with hook" lf.txt
> +
> +'
> +test_expect_success 'crlf with hook white-space failure' '
> +
> +	echo "bar \r" >> crlf.txt &&
> +	git add crlf.txt &&
> +	! git commit -m "crlf with hook" crlf.txt
> +
> +'
> +
> +test_done
> diff --git a/templates/hooks--pre-commit b/templates/hooks--pre-commit
> index b25dce6..335ca09 100644
> --- a/templates/hooks--pre-commit
> +++ b/templates/hooks--pre-commit
> @@ -55,8 +55,14 @@ perl -e '
> 	if (s/^\+//) {
> 	    $lineno++;
> 	    chomp;
> -	    if (/\s$/) {
> -		bad_line("trailing whitespace", $_);
> +	    if (/\r$/) {

Wouldn't it be less redundant to do a test for \s\r$ here instead of
testing for \r$ and then \s\r$?

> +		if (/\s\r$/) {
> +		    bad_line("trailing whitespace", $_);
> +		}
> +	    } else {
> +		if (/\s$/) {
> +		    bad_line("trailing whitespace", $_);
> +		}
> 	    }
> 	    if (/^\s* \t/) {
> 		bad_line("indent SP followed by a TAB", $_);
> 

-- 
Ian Hilt
Ian.Hilt (at) gmx.com
GnuPG key: 0x4AFC1EE3