Re: [PATCH 1/6] templates: Fewer subprocesses in pre-commit hook

2013-06-10 Thread Jeff King
On Mon, Jun 10, 2013 at 08:36:00PM +0200, Richard Hartmann wrote:

> Spawning a new subprocess for every line printed is inefficient.
> Thus spawn only one instance of `echo`.

Most modern shells have "echo" as a built-in these days, and do not fork
at all to run it. E.g., try "strace sh -c 'echo foo'" with your shell of
choice; neither dash nor bash will fork at all.

IMHO the indentation issues make the end result of your patch less
readable (and here-doc with cat is more readable, but actually
_increases_ the number of processes, since cat is not usually a
built-in). So I'd be in favor of keeping it as-is.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/6] templates: Fewer subprocesses in pre-commit hook

2013-06-10 Thread Richard Hartmann
Hi Junio,

if you want a new patch, just say the word.


Richard
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/6] templates: Fewer subprocesses in pre-commit hook

2013-06-10 Thread Junio C Hamano
Richard Hartmann  writes:

> Spawning a new subprocess for every line printed is inefficient.
> Thus spawn only one instance of `echo`.
>
> Signed-off-by: Richard Hartmann 
> ---
>  templates/hooks--pre-commit.sample |   24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/templates/hooks--pre-commit.sample 
> b/templates/hooks--pre-commit.sample
> index 18c4829..126ae13 100755
> --- a/templates/hooks--pre-commit.sample
> +++ b/templates/hooks--pre-commit.sample
> @@ -31,18 +31,18 @@ if [ "$allownonascii" != "true" ] &&
>   test $(git diff --cached --name-only --diff-filter=A -z $against |
> LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0
>  then
> - echo "Error: Attempt to add a non-ascii file name."
> - echo
> - echo "This can cause problems if you want to work"
> - echo "with people on other platforms."
> - echo
> - echo "To be portable it is advisable to rename the file ..."
> - echo
> - echo "If you know what you are doing you can disable this"
> - echo "check using:"
> - echo
> - echo "  git config hooks.allownonascii true"
> - echo
> + echo 'Error: Attempt to add a non-ascii file name.
> +
> +This can cause problems if you want to work
> +with people on other platforms.
> +
> +To be portable it is advisable to rename the file.
> +
> +If you know what you are doing you can disable this
> +check using:
> +
> +  git config hooks.allownonascii true
> +'
>   exit 1
>  fi

Thanks.
Writing it as a single here-text

cat <<-EOF
Error: Attempt to...

the message body that is
multi-line
EOF

might make it easier for people who want to activate and customize
the message, but honestly this is a borderline "Meh" at least to me.

Will take a look at other patches first before further commenting on
this.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/6] templates: Fewer subprocesses in pre-commit hook

2013-06-10 Thread Richard Hartmann
Spawning a new subprocess for every line printed is inefficient.
Thus spawn only one instance of `echo`.

Signed-off-by: Richard Hartmann 
---
 templates/hooks--pre-commit.sample |   24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/templates/hooks--pre-commit.sample 
b/templates/hooks--pre-commit.sample
index 18c4829..126ae13 100755
--- a/templates/hooks--pre-commit.sample
+++ b/templates/hooks--pre-commit.sample
@@ -31,18 +31,18 @@ if [ "$allownonascii" != "true" ] &&
test $(git diff --cached --name-only --diff-filter=A -z $against |
  LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0
 then
-   echo "Error: Attempt to add a non-ascii file name."
-   echo
-   echo "This can cause problems if you want to work"
-   echo "with people on other platforms."
-   echo
-   echo "To be portable it is advisable to rename the file ..."
-   echo
-   echo "If you know what you are doing you can disable this"
-   echo "check using:"
-   echo
-   echo "  git config hooks.allownonascii true"
-   echo
+   echo 'Error: Attempt to add a non-ascii file name.
+
+This can cause problems if you want to work
+with people on other platforms.
+
+To be portable it is advisable to rename the file.
+
+If you know what you are doing you can disable this
+check using:
+
+  git config hooks.allownonascii true
+'
exit 1
 fi
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html