Re: [PATCH] Documentation/githooks: Explain pre-rebase parameters

"W. Trevor King" [off-list ref] writes:

From: "W. Trevor King" <redacted>

Descriptions borrowed from templates/hooks--pre-rebase.sample.

Signed-off-by: W. Trevor King <redacted>
---
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.

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).

On Tue, Feb 19, 2013 at 11:08:29AM -0800, Junio C Hamano wrote:

"W. Trevor King" [off-list ref] writes:

quoted

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.

quoted

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

From: "W. Trevor King" <redacted>

Descriptions borrowed from templates/hooks--pre-rebase.sample.

Signed-off-by: W. Trevor King <redacted>
---
Changes from v1:
* Replaced "empty" with "missing" for second parameter.

 Documentation/githooks.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index b9003fe..dfd5959 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 missing when rebasing the current branch.
 
 post-checkout
 ~~~~~~~~~~~~~

-- 
1.8.2.rc0.16.g20a599e