Re: [PATCH] Documentation/githooks: Explain pre-rebase parameters
On Tue, Feb 19, 2013 at 11:08:29AM -0800, Junio C Hamano wrote:
W. Trevor King wk...@tremily.us writes:
Also, it appears that the `git-rebase--*.sh` handlers don't use the
pre-rebase hook. Is this intentional?
The codeflow of git-rebase front-end, when you start rebasing, will
call run_pre_rebase_hook before calling run_specific_rebase. It
will be redundant for handlers to then call it again, no?
In rebase --continue and later steps, you would not want to see
the hook trigger.
Ah, that makes sense.
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index b9003fe..bc837c6 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -140,9 +140,10 @@ the outcome of 'git commit'.
pre-rebase
~~
-This hook is called by 'git rebase' and can be used to prevent a branch
-from getting rebased.
-
+This hook is called by 'git rebase' and can be used to prevent a
+branch from getting rebased. The hook takes two parameters: the
+upstream the series was forked from and the branch being rebased. The
+second parameter will be empty when rebasing the current branch.
Technically this is incorrect.
We call it with one or two parameters, and sometimes the second
parameter is _missing_, which is different from calling with an
empty string. For a script written in some scripting languages like
shell and perl, the distinction may not matter (i.e. $2 and $ARGV[1]
will be an empty string when stringified) but not all (accessing
sys.argv[2] may give you an IndexError in Python).
Will fix in v2.
Since $upstream_arg will always be set, would it make sense to change
the `${1+$@}` syntax in run_pre_rebase_hook() to a plain $@?
Cheers,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: [PATCH] Documentation/githooks: Explain pre-rebase parameters
W. Trevor King wk...@tremily.us writes:
Since $upstream_arg will always be set, would it make sense to change
the `${1+$@}` syntax in run_pre_rebase_hook() to a plain $@?
I suspect that there no longer is a need for ${1+$@} in today's
world even when you do not have arguments, and it certainly is fine
if you want to update that particular instance in the function with
a single caller that calls it with 1 or 2 arguments, especially if
you are updating the code in the vicinity.
I however do not think it is worth blindly replacing them tree-wide
just for the sake of changing them. The upside of helping beginning
shell programers by possibly better readability does not look great,
compared to the downside of possibly breaking somebody who is still
on a broken shell that the old idiom is still helping.
--
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] Documentation/githooks: Explain pre-rebase parameters
From: W. Trevor King wk...@tremily.us
Descriptions borrowed from templates/hooks--pre-rebase.sample.
Signed-off-by: W. Trevor King wk...@tremily.us
---
I'm not 100% convinced about this, because the git-rebase.sh uses:
$GIT_DIR/hooks/pre-rebase ${1+$@}
I haven't been able to find documentation for the ${1+$@} syntax.
Is it in POSIX? It's not in the Bash manual:
$ man bash | grep '\${.*[+]'
(${BASH_SOURCE[$i+1]}) where ${FUNCNAME[$i]} was called (or
${BASH_SOURCE[$i+1]}.
${BASH_SOURCE[$i+1]} at line number ${BASH_LINENO[$i]}. The
${parameter:+word}
In my local tests, it seems equivalent to $@.
Also, it appears that the `git-rebase--*.sh` handlers don't use the
pre-rebase hook. Is this intentional?
Cheers,
Trevor
Documentation/githooks.txt | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index b9003fe..bc837c6 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -140,9 +140,10 @@ the outcome of 'git commit'.
pre-rebase
~~
-This hook is called by 'git rebase' and can be used to prevent a branch
-from getting rebased.
-
+This hook is called by 'git rebase' and can be used to prevent a
+branch from getting rebased. The hook takes two parameters: the
+upstream the series was forked from and the branch being rebased. The
+second parameter will be empty when rebasing the current branch.
post-checkout
~
--
1.8.1.336.g94702dd
--
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] Documentation/githooks: Explain pre-rebase parameters
W. Trevor King wk...@tremily.us writes:
I'm not 100% convinced about this, because the git-rebase.sh uses:
$GIT_DIR/hooks/pre-rebase ${1+$@}
I haven't been able to find documentation for the ${1+$@} syntax.
Is it in POSIX? It's not in the Bash manual:
[...]
In my local tests, it seems equivalent to $@.
It's definitely in the bash manual and POSIX[1]: it's a special case of
the ${parameter+word} expansion.
${parameter:+word}
Use Alternate Value. If parameter is null or unset, nothing is
substituted, otherwise the expansion of word is substituted.
plus
... bash tests for a parameter that is unset or null. Omitting the
colon results in a test only for a parameter that is unset.
IIRC this particular usage was designed to suppress warnings about unset
variables.
Footnotes:
[1] http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
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] Documentation/githooks: Explain pre-rebase parameters
On Tue, Feb 19, 2013 at 02:17:43PM +0100, Thomas Rast wrote:
W. Trevor King wk...@tremily.us writes:
I haven't been able to find documentation for the ${1+$@} syntax.
Is it in POSIX? It's not in the Bash manual:
[...]
In my local tests, it seems equivalent to $@.
It's definitely in the bash manual and POSIX[1]: it's a special case of
the ${parameter+word} expansion.
I need to read more carefully ;). There's even a nice table in the
POSIX specs…
Thanks,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: [PATCH] Documentation/githooks: Explain pre-rebase parameters
Thomas Rast tr...@student.ethz.ch writes:
$GIT_DIR/hooks/pre-rebase ${1+$@}
...
IIRC this particular usage was designed to suppress warnings about unset
variables.
This is an old-timer's habit to work around buggy implementations of
Bourne shells where they failed to expand $@ to nothing when there
is no parameters, feeding a single empty argument to the command.
By explicitly writing ${1+...}, the construct makes sure that when
we have no parameter (i.e. $1 is unset) we do not even look at $@.
It is equivalent to $@ in correctly implemented shells.
--
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] Documentation/githooks: Explain pre-rebase parameters
W. Trevor King wk...@tremily.us writes:
From: W. Trevor King wk...@tremily.us
Descriptions borrowed from templates/hooks--pre-rebase.sample.
Signed-off-by: W. Trevor King wk...@tremily.us
---
I'm not 100% convinced about this, because the git-rebase.sh uses:
$GIT_DIR/hooks/pre-rebase ${1+$@}
I haven't been able to find documentation for the ${1+$@} syntax.
Is it in POSIX? It's not in the Bash manual:
$ man bash | grep '\${.*[+]'
(${BASH_SOURCE[$i+1]}) where ${FUNCNAME[$i]} was called (or
${BASH_SOURCE[$i+1]}.
${BASH_SOURCE[$i+1]} at line number ${BASH_LINENO[$i]}. The
${parameter:+word}
In my local tests, it seems equivalent to $@.
Also, it appears that the `git-rebase--*.sh` handlers don't use the
pre-rebase hook. Is this intentional?
The codeflow of git-rebase front-end, when you start rebasing, will
call run_pre_rebase_hook before calling run_specific_rebase. It
will be redundant for handlers to then call it again, no?
In rebase --continue and later steps, you would not want to see
the hook trigger.
Documentation/githooks.txt | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index b9003fe..bc837c6 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -140,9 +140,10 @@ the outcome of 'git commit'.
pre-rebase
~~
-This hook is called by 'git rebase' and can be used to prevent a branch
-from getting rebased.
-
+This hook is called by 'git rebase' and can be used to prevent a
+branch from getting rebased. The hook takes two parameters: the
+upstream the series was forked from and the branch being rebased. The
+second parameter will be empty when rebasing the current branch.
Technically this is incorrect.
We call it with one or two parameters, and sometimes the second
parameter is _missing_, which is different from calling with an
empty string. For a script written in some scripting languages like
shell and perl, the distinction may not matter (i.e. $2 and $ARGV[1]
will be an empty string when stringified) but not all (accessing
sys.argv[2] may give you an IndexError in Python).
--
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
