{"thread":{"id":"32938","subject":"[PATCH] Documentation/githooks: Explain pre-rebase parameters","startedAt":"2013-02-19T11:03:24Z","lastAt":"2013-02-24T08:13:25Z","messageCount":11,"participants":["W. Trevor King","Thomas Rast","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"209808","messageId":"c19c03f51d71a58fa3795f665fe4a4c0461fa58f.1361271116.git.wking@tremily.us","threadId":"32938","inReplyTo":null,"subject":"[PATCH] Documentation/githooks: Explain pre-rebase parameters","fromName":"W. Trevor King","fromEmail":"wking@tremily.us","sentAt":"2013-02-19T11:03:24Z","receivedAt":"2013-02-19T11:03:24Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"From: \"W. Trevor King\" \n\nDescriptions borrowed from templates/hooks--pre-rebase.sample.\n\nSigned-off-by: W. Trevor King \n---\nI'm not 100% convinced about this, because the git-rebase.sh uses:\n\n \"$GIT_DIR/hooks/pre-rebase\" ${1+\"$@\"}\n\nI haven't been able to find documentation for the ${1+\"$@\"} syntax.\nIs it in POSIX? It's not in the Bash manual:\n\n $ man bash | grep '\\${.*[+]'\n (${BASH_SOURCE[$i+1]}) where ${FUNCNAME[$i]} was called (or\n ${BASH_SOURCE[$i+1]}.\n ${BASH_SOURCE[$i+1]} at line number ${BASH_LINENO[$i]}. The\n ${parameter:+word}\n\nIn my local tests, it seems equivalent to \"$@\".\n\nAlso, it appears that the `git-rebase--*.sh` handlers don't use the\npre-rebase hook. Is this intentional?\n\nCheers,\nTrevor\n\n Documentation/githooks.txt | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex b9003fe..bc837c6 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -140,9 +140,10 @@ the outcome of 'git commit'.\n pre-rebase\n ~~~~~~~~~~\n \n-This hook is called by 'git rebase' and can be used to prevent a branch\n-from getting rebased.\n-\n+This hook is called by 'git rebase' and can be used to prevent a\n+branch from getting rebased. The hook takes two parameters: the\n+upstream the series was forked from and the branch being rebased. The\n+second parameter will be empty when rebasing the current branch.\n \n post-checkout\n ~~~~~~~~~~~~~\n-- \n1.8.1.336.g94702dd\n"},{"id":"209818","messageId":"878v6ksars.fsf@pctrast.inf.ethz.ch","threadId":"32938","inReplyTo":"c19c03f51d71a58fa3795f665fe4a4c0461fa58f.1361271116.git.wking@tremily.us","subject":"Re: [PATCH] Documentation/githooks: Explain pre-rebase parameters","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2013-02-19T13:17:43Z","receivedAt":"2013-02-19T13:17:43Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"\"W. Trevor King\" writes:\n\n> I'm not 100% convinced about this, because the git-rebase.sh uses:\n>\n> \"$GIT_DIR/hooks/pre-rebase\" ${1+\"$@\"}\n>\n> I haven't been able to find documentation for the ${1+\"$@\"} syntax.\n> Is it in POSIX? It's not in the Bash manual:\n[...]\n> In my local tests, it seems equivalent to \"$@\".\n\nIt's definitely in the bash manual and POSIX[1]: it's a special case of\nthe ${parameter+word} expansion.\n\n ${parameter:+word}\n Use Alternate Value. If parameter is null or unset, nothing is\n substituted, otherwise the expansion of word is substituted.\n\nplus\n\n ... bash tests for a parameter that is unset or null. Omitting the\n colon results in a test only for a parameter that is unset.\n\nIIRC this particular usage was designed to suppress warnings about unset\nvariables.\n\n\nFootnotes: \n[1] http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"209819","messageId":"20130219132331.GD5125@odin.tremily.us","threadId":"32938","inReplyTo":"878v6ksars.fsf@pctrast.inf.ethz.ch","subject":"Re: [PATCH] Documentation/githooks: Explain pre-rebase parameters","fromName":"W. Trevor King","fromEmail":"wking@tremily.us","sentAt":"2013-02-19T13:23:31Z","receivedAt":"2013-02-19T13:23:31Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"On Tue, Feb 19, 2013 at 02:17:43PM +0100, Thomas Rast wrote:\n> \"W. Trevor King\" writes:\n> > I haven't been able to find documentation for the ${1+\"$@\"} syntax.\n> > Is it in POSIX? It's not in the Bash manual:\n> [...]\n> > In my local tests, it seems equivalent to \"$@\".\n> \n> It's definitely in the bash manual and POSIX[1]: it's a special case of\n> the ${parameter+word} expansion.\n\nI need to read more carefully ;). There's even a nice table in the\nPOSIX specs…\n\nThanks,\nTrevor\n\n-- \nThis email may be signed or encrypted with GnuPG (http://www.gnupg.org).\nFor more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy\n"},{"id":"209838","messageId":"7vliak6xop.fsf@alter.siamese.dyndns.org","threadId":"32938","inReplyTo":"878v6ksars.fsf@pctrast.inf.ethz.ch","subject":"Re: [PATCH] Documentation/githooks: Explain pre-rebase parameters","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-19T17:05:58Z","receivedAt":"2013-02-19T17:05:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n>> \"$GIT_DIR/hooks/pre-rebase\" ${1+\"$@\"}\n> ...\n> IIRC this particular usage was designed to suppress warnings about unset\n> variables.\n\nThis is an old-timer's habit to work around buggy implementations of\nBourne shells where they failed to expand \"$@\" to nothing when there\nis no parameters, feeding a single empty argument to the command.\nBy explicitly writing ${1+...}, the construct makes sure that when\nwe have no parameter (i.e. $1 is unset) we do not even look at \"$@\".\n\nIt is equivalent to \"$@\" in correctly implemented shells.\n"},{"id":"209853","messageId":"7vk3q45dg2.fsf@alter.siamese.dyndns.org","threadId":"32938","inReplyTo":"c19c03f51d71a58fa3795f665fe4a4c0461fa58f.1361271116.git.wking@tremily.us","subject":"Re: [PATCH] Documentation/githooks: Explain pre-rebase parameters","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-19T19:08:29Z","receivedAt":"2013-02-19T19:08:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"W. Trevor King\" writes:\n\n> From: \"W. Trevor King\" \n>\n> Descriptions borrowed from templates/hooks--pre-rebase.sample.\n>\n> Signed-off-by: W. Trevor King \n> ---\n> I'm not 100% convinced about this, because the git-rebase.sh uses:\n>\n> \"$GIT_DIR/hooks/pre-rebase\" ${1+\"$@\"}\n>\n> I haven't been able to find documentation for the ${1+\"$@\"} syntax.\n> Is it in POSIX? It's not in the Bash manual:\n>\n> $ man bash | grep '\\${.*[+]'\n> (${BASH_SOURCE[$i+1]}) where ${FUNCNAME[$i]} was called (or\n> ${BASH_SOURCE[$i+1]}.\n> ${BASH_SOURCE[$i+1]} at line number ${BASH_LINENO[$i]}. The\n> ${parameter:+word}\n>\n> In my local tests, it seems equivalent to \"$@\".\n>\n> Also, it appears that the `git-rebase--*.sh` handlers don't use the\n> pre-rebase hook. Is this intentional?\n\nThe codeflow of git-rebase front-end, when you start rebasing, will\ncall run_pre_rebase_hook before calling run_specific_rebase. It\nwill be redundant for handlers to then call it again, no?\n\nIn \"rebase --continue\" and later steps, you would not want to see\nthe hook trigger.\n\n> Documentation/githooks.txt | 7 ++++---\n> 1 file changed, 4 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> index b9003fe..bc837c6 100644\n> --- a/Documentation/githooks.txt\n> +++ b/Documentation/githooks.txt\n> @@ -140,9 +140,10 @@ the outcome of 'git commit'.\n> pre-rebase\n> ~~~~~~~~~~\n> \n> -This hook is called by 'git rebase' and can be used to prevent a branch\n> -from getting rebased.\n> -\n> +This hook is called by 'git rebase' and can be used to prevent a\n> +branch from getting rebased. The hook takes two parameters: the\n> +upstream the series was forked from and the branch being rebased. The\n> +second parameter will be empty when rebasing the current branch.\n\nTechnically this is incorrect.\n\nWe call it with one or two parameters, and sometimes the second\nparameter is _missing_, which is different from calling with an\nempty string. For a script written in some scripting languages like\nshell and perl, the distinction may not matter (i.e. $2 and $ARGV[1]\nwill be an empty string when stringified) but not all (accessing\nsys.argv[2] may give you an IndexError in Python).\n"},{"id":"209912","messageId":"20130220163621.GI14102@odin.tremily.us","threadId":"32938","inReplyTo":"7vk3q45dg2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Documentation/githooks: Explain pre-rebase parameters","fromName":"W. Trevor King","fromEmail":"wking@tremily.us","sentAt":"2013-02-20T16:36:21Z","receivedAt":"2013-02-20T16:36:21Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"On Tue, Feb 19, 2013 at 11:08:29AM -0800, Junio C Hamano wrote:\n> \"W. Trevor King\" writes:\n> > Also, it appears that the `git-rebase--*.sh` handlers don't use the\n> > pre-rebase hook. Is this intentional?\n> \n> The codeflow of git-rebase front-end, when you start rebasing, will\n> call run_pre_rebase_hook before calling run_specific_rebase. It\n> will be redundant for handlers to then call it again, no?\n> \n> In \"rebase --continue\" and later steps, you would not want to see\n> the hook trigger.\n\nAh, that makes sense.\n\n> > diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\n> > index b9003fe..bc837c6 100644\n> > --- a/Documentation/githooks.txt\n> > +++ b/Documentation/githooks.txt\n> > @@ -140,9 +140,10 @@ the outcome of 'git commit'.\n> > pre-rebase\n> > ~~~~~~~~~~\n> > \n> > -This hook is called by 'git rebase' and can be used to prevent a branch\n> > -from getting rebased.\n> > -\n> > +This hook is called by 'git rebase' and can be used to prevent a\n> > +branch from getting rebased. The hook takes two parameters: the\n> > +upstream the series was forked from and the branch being rebased. The\n> > +second parameter will be empty when rebasing the current branch.\n> \n> Technically this is incorrect.\n> \n> We call it with one or two parameters, and sometimes the second\n> parameter is _missing_, which is different from calling with an\n> empty string. For a script written in some scripting languages like\n> shell and perl, the distinction may not matter (i.e. $2 and $ARGV[1]\n> will be an empty string when stringified) but not all (accessing\n> sys.argv[2] may give you an IndexError in Python).\n\nWill fix in v2.\n\nSince $upstream_arg will always be set, would it make sense to change\nthe `${1+\"$@\"}` syntax in run_pre_rebase_hook() to a plain \"$@\"?\n\nCheers,\nTrevor\n\n-- \nThis email may be signed or encrypted with GnuPG (http://www.gnupg.org).\nFor more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy\n"},{"id":"209914","messageId":"7vwqu2yk42.fsf@alter.siamese.dyndns.org","threadId":"32938","inReplyTo":"20130220163621.GI14102@odin.tremily.us","subject":"Re: [PATCH] Documentation/githooks: Explain pre-rebase parameters","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-20T17:23:57Z","receivedAt":"2013-02-20T17:23:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"W. Trevor King\" writes:\n\n> Since $upstream_arg will always be set, would it make sense to change\n> the `${1+\"$@\"}` syntax in run_pre_rebase_hook() to a plain \"$@\"?\n\nI suspect that there no longer is a need for ${1+\"$@\"} in today's\nworld even when you do not have arguments, and it certainly is fine\nif you want to update that particular instance in the function with\na single caller that calls it with 1 or 2 arguments, especially if\nyou are updating the code in the vicinity.\n\nI however do not think it is worth blindly replacing them tree-wide\njust for the sake of changing them. The upside of helping beginning\nshell programers by possibly better readability does not look great,\ncompared to the downside of possibly breaking somebody who is still\non a broken shell that the old idiom is still helping.\n"},{"id":"210091","messageId":"c8b19dc074a81b009399ff1011102737761658ec.1361633106.git.wking@tremily.us","threadId":"32938","inReplyTo":"20130220163621.GI14102@odin.tremily.us","subject":"[PATCH v2] Documentation/githooks: Explain pre-rebase parameters","fromName":"W. Trevor King","fromEmail":"wking@tremily.us","sentAt":"2013-02-23T15:27:39Z","receivedAt":"2013-02-23T15:27:39Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"From: \"W. Trevor King\" \n\nDescriptions borrowed from templates/hooks--pre-rebase.sample.\n\nSigned-off-by: W. Trevor King \n---\nChanges from v1:\n* Replaced \"empty\" with \"missing\" for second parameter.\n\n Documentation/githooks.txt | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/githooks.txt b/Documentation/githooks.txt\nindex b9003fe..dfd5959 100644\n--- a/Documentation/githooks.txt\n+++ b/Documentation/githooks.txt\n@@ -140,9 +140,10 @@ the outcome of 'git commit'.\n pre-rebase\n ~~~~~~~~~~\n \n-This hook is called by 'git rebase' and can be used to prevent a branch\n-from getting rebased.\n-\n+This hook is called by 'git rebase' and can be used to prevent a\n+branch from getting rebased. The hook takes two parameters: the\n+upstream the series was forked from and the branch being rebased. The\n+second parameter will be missing when rebasing the current branch.\n \n post-checkout\n ~~~~~~~~~~~~~\n-- \n1.8.2.rc0.16.g20a599e\n"},{"id":"210107","messageId":"7vobfa7mko.fsf@alter.siamese.dyndns.org","threadId":"32938","inReplyTo":"c8b19dc074a81b009399ff1011102737761658ec.1361633106.git.wking@tremily.us","subject":"Re: [PATCH v2] Documentation/githooks: Explain pre-rebase parameters","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-23T21:21:59Z","receivedAt":"2013-02-23T21:21:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"W. Trevor King\" writes:\n\n> +This hook is called by 'git rebase' and can be used to prevent a\n> +branch from getting rebased. The hook takes two parameters: the\n> +upstream the series was forked from and the branch being rebased. The\n> +second parameter will be missing when rebasing the current branch.\n\ntakes one or two parameters?\n\nOther than that, looks good to me, but it took me two readings to\nnotice where these two parameters are described. I have a feeling\nthat a comma s/forked from and/forked from, and/; might make them a\nbit more spottable, but others may have better suggestions to make\nthem stand out more.\n\nThanks, will queue.\n\n> \n> post-checkout\n> ~~~~~~~~~~~~~\n"},{"id":"210110","messageId":"20130223213513.GF1361@odin.tremily.us","threadId":"32938","inReplyTo":"7vobfa7mko.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] Documentation/githooks: Explain pre-rebase parameters","fromName":"W. Trevor King","fromEmail":"wking@tremily.us","sentAt":"2013-02-23T21:35:13Z","receivedAt":"2013-02-23T21:35:13Z","isPatch":true,"sender":{"key":"wking@tremily.us","avatar":"https://avatars.githubusercontent.com/u/209920?v=4"},"body":"On Sat, Feb 23, 2013 at 01:21:59PM -0800, Junio C Hamano wrote:\n> \"W. Trevor King\" writes:\n> \n> > +This hook is called by 'git rebase' and can be used to prevent a\n> > +branch from getting rebased. The hook takes two parameters: the\n> > +upstream the series was forked from and the branch being rebased. The\n> > +second parameter will be missing when rebasing the current branch.\n> \n> takes one or two parameters?\n>\n> Other than that, looks good to me, but it took me two readings to\n> notice where these two parameters are described. I have a feeling\n> that a comma s/forked from and/forked from, and/; might make them a\n> bit more spottable, but others may have better suggestions to make\n> them stand out more.\n\nHow about:\n\n The hook may be called with one or two parameters. The first\n parameter is the upstream from which the series was forked. The\n second parameter is the branch being rebased, and is not set when\n rebasing the current branch.\n\n-- \nThis email may be signed or encrypted with GnuPG (http://www.gnupg.org).\nFor more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy\n"},{"id":"210170","messageId":"7v621i6sey.fsf@alter.siamese.dyndns.org","threadId":"32938","inReplyTo":"20130223213513.GF1361@odin.tremily.us","subject":"Re: [PATCH v2] Documentation/githooks: Explain pre-rebase parameters","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-24T08:13:25Z","receivedAt":"2013-02-24T08:13:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"W. Trevor King\" writes:\n\n> On Sat, Feb 23, 2013 at 01:21:59PM -0800, Junio C Hamano wrote:\n>> \"W. Trevor King\" writes:\n>> \n>> > +This hook is called by 'git rebase' and can be used to prevent a\n>> > +branch from getting rebased. The hook takes two parameters: the\n>> > +upstream the series was forked from and the branch being rebased. The\n>> > +second parameter will be missing when rebasing the current branch.\n>> \n>> takes one or two parameters?\n>>\n>> Other than that, looks good to me, but it took me two readings to\n>> notice where these two parameters are described. I have a feeling\n>> that a comma s/forked from and/forked from, and/; might make them a\n>> bit more spottable, but others may have better suggestions to make\n>> them stand out more.\n>\n> How about:\n>\n> The hook may be called with one or two parameters. The first\n> parameter is the upstream from which the series was forked. The\n> second parameter is the branch being rebased, and is not set when\n> rebasing the current branch.\n\nMuch nicer. Thanks.\n"}]}