# [PATCH] hooks--pre-commit: detect non-ASCII when renaming 5 messages from 2022-07-14 to 2023-12-03. Participants: Julian Prein via GitGitGadget, Junio C Hamano. Thread: https://gitlist.dev/t/58163 ## Julian Prein via GitGitGadget, 2022-07-14 22:17 Subject: [PATCH] hooks--pre-commit: detect non-ASCII when renaming Message-ID: URL: https://gitlist.dev/e/pull.1291.git.git.1657837073372.gitgitgadget%40gmail.com ``` From: Julian Prein Currently the diff-filter that is used to check for non-ASCII characters in filenames only checks new additions. Extend the diff-filter in the pre-commit sample to include `CR` as well. This way non-ASCII character in filenames are detected on a rename/copy as well. Signed-off-by: Julian Prein --- hooks--pre-commit: detect non-ASCII when renaming Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1291%2Fdruckdev%2Fpre-commit-renames-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1291/druckdev/pre-commit-renames-v1 Pull-Request: https://github.com/git/git/pull/1291 templates/hooks--pre-commit.sample | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample index e144712c85c..95c327832da 100755 --- a/templates/hooks--pre-commit.sample +++ b/templates/hooks--pre-commit.sample @@ -28,7 +28,7 @@ if [ "$allownonascii" != "true" ] && # Note that the use of brackets around a tr range is ok here, (it's # even required, for portability to Solaris 10's /usr/bin/tr), since # the square bracket bytes happen to fall in the designated range. - test $(git diff --cached --name-only --diff-filter=A -z $against | + test $(git diff --cached --name-only --diff-filter=ACR -z $against | LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0 then cat <<\EOF base-commit: bbea4dcf42b28eb7ce64a6306cdde875ae5d09ca -- gitgitgadget ``` ## Junio C Hamano, 2022-07-14 23:03 Subject: Re: [PATCH] hooks--pre-commit: detect non-ASCII when renaming Message-ID: URL: https://gitlist.dev/e/xmqqedyn2t6e.fsf%40gitster.g In-Reply-To: ``` "Julian Prein via GitGitGadget" writes: > From: Julian Prein > > Currently the diff-filter that is used to check for non-ASCII characters > in filenames only checks new additions. > > Extend the diff-filter in the pre-commit sample to include `CR` as well. > This way non-ASCII character in filenames are detected on a rename/copy > as well. It would probably be a better implementation to disable the rename detection, instead of adding Copies and Renames. > Signed-off-by: Julian Prein > --- > hooks--pre-commit: detect non-ASCII when renaming > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1291%2Fdruckdev%2Fpre-commit-renames-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1291/druckdev/pre-commit-renames-v1 > Pull-Request: https://github.com/git/git/pull/1291 > > templates/hooks--pre-commit.sample | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample > index e144712c85c..95c327832da 100755 > --- a/templates/hooks--pre-commit.sample > +++ b/templates/hooks--pre-commit.sample > @@ -28,7 +28,7 @@ if [ "$allownonascii" != "true" ] && > # Note that the use of brackets around a tr range is ok here, (it's > # even required, for portability to Solaris 10's /usr/bin/tr), since > # the square bracket bytes happen to fall in the designated range. > - test $(git diff --cached --name-only --diff-filter=A -z $against | > + test $(git diff --cached --name-only --diff-filter=ACR -z $against | > LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0 > then > cat <<\EOF > > base-commit: bbea4dcf42b28eb7ce64a6306cdde875ae5d09ca ``` ## Junio C Hamano, 2022-07-14 23:08 Subject: Re: [PATCH] hooks--pre-commit: detect non-ASCII when renaming Message-ID: URL: https://gitlist.dev/e/xmqqa69b2syj.fsf%40gitster.g In-Reply-To: ``` Junio C Hamano writes: > "Julian Prein via GitGitGadget" writes: > >> From: Julian Prein >> >> Currently the diff-filter that is used to check for non-ASCII characters >> in filenames only checks new additions. >> >> Extend the diff-filter in the pre-commit sample to include `CR` as well. >> This way non-ASCII character in filenames are detected on a rename/copy >> as well. > > It would probably be a better implementation to disable the rename > detection, instead of adding Copies and Renames. That is ... >> - test $(git diff --cached --name-only --diff-filter=A -z $against | ... instead of using the "git diff" Porcelain, we can probably use the "git diff-index" plumbing command here, which will not be affected by end-user configuration or by the fact that recent "git diff" turns on the rename detection by default. ``` ## Julian Prein via GitGitGadget, 2023-11-30 16:13 Subject: [PATCH v2] hooks--pre-commit: detect non-ASCII when renaming Message-ID: URL: https://gitlist.dev/e/pull.1291.v2.git.git.1701360836307.gitgitgadget%40gmail.com In-Reply-To: ``` From: Julian Prein When diff.renames is turned on, the diff-filter will not return renamed files (or copied ones with diff.renames=copy) and potential non-ASCII characters would not be caught by this hook. Use the plumbing command diff-index instead of the porcelain one to not be affected by diff.rename. Signed-off-by: Julian Prein --- hooks--pre-commit: detect non-ASCII when renaming A bit later than I expected, but here is v2. Changes since v1: * Switched to using diff-index and back to just the A filter as suggested by Junio C Hamano Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1291%2Fdruckdev%2Fpre-commit-renames-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1291/druckdev/pre-commit-renames-v2 Pull-Request: https://github.com/git/git/pull/1291 Range-diff vs v1: 1: 101f327a040 ! 1: 1f6ca0dd3eb hooks--pre-commit: detect non-ASCII when renaming @@ Metadata ## Commit message ## hooks--pre-commit: detect non-ASCII when renaming - Currently the diff-filter that is used to check for non-ASCII characters - in filenames only checks new additions. + When diff.renames is turned on, the diff-filter will not return renamed + files (or copied ones with diff.renames=copy) and potential non-ASCII + characters would not be caught by this hook. - Extend the diff-filter in the pre-commit sample to include `CR` as well. - This way non-ASCII character in filenames are detected on a rename/copy - as well. + Use the plumbing command diff-index instead of the porcelain one to not + be affected by diff.rename. Signed-off-by: Julian Prein @@ templates/hooks--pre-commit.sample: if [ "$allownonascii" != "true" ] && # even required, for portability to Solaris 10's /usr/bin/tr), since # the square bracket bytes happen to fall in the designated range. - test $(git diff --cached --name-only --diff-filter=A -z $against | -+ test $(git diff --cached --name-only --diff-filter=ACR -z $against | ++ test $(git diff-index --cached --name-only --diff-filter=A -z $against | LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0 then cat <<\EOF templates/hooks--pre-commit.sample | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/hooks--pre-commit.sample b/templates/hooks--pre-commit.sample index e144712c85c..29ed5ee486a 100755 --- a/templates/hooks--pre-commit.sample +++ b/templates/hooks--pre-commit.sample @@ -28,7 +28,7 @@ if [ "$allownonascii" != "true" ] && # Note that the use of brackets around a tr range is ok here, (it's # even required, for portability to Solaris 10's /usr/bin/tr), since # the square bracket bytes happen to fall in the designated range. - test $(git diff --cached --name-only --diff-filter=A -z $against | + test $(git diff-index --cached --name-only --diff-filter=A -z $against | LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0 then cat <<\EOF base-commit: 61a22ddaf0626111193a17ac12f366bd6d167dff -- gitgitgadget ``` ## Junio C Hamano, 2023-12-03 13:15 Subject: Re: [PATCH v2] hooks--pre-commit: detect non-ASCII when renaming Message-ID: URL: https://gitlist.dev/e/xmqqplzn4eci.fsf%40gitster.g In-Reply-To: ``` "Julian Prein via GitGitGadget" writes: > From: Julian Prein > > When diff.renames is turned on, the diff-filter will not return renamed > files (or copied ones with diff.renames=copy) and potential non-ASCII > characters would not be caught by this hook. > > Use the plumbing command diff-index instead of the porcelain one to not > be affected by diff.rename. Makes sense. An obvious alternative would be to pass "--no-renames" and keep using the Porcelain "git diff", but this is how the plumbing "diff-index" and friends are meant to be used. Looking good. Will queue. Thanks. ```