On Mon, Feb 20, 2012 at 08:58, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Philip Jägenstedt <philip@xxxxxxxxxx> writes:
>
>> Complete <name> only for set-url. For set-branches and
>> set-head, complete <name> and <branch> over the network,
>> like e.g. git pull already does.
>>
>> Signed-off-by: Philip Jägenstedt <philip@xxxxxxxxxx>
>
> You addressed your patch to Shawn, who originally wrote this, but
>
> "git shortlog -n -s --no-merges --since=9.months pu contrib/completion"
>
> indicates that he no longer is involved in enhancing this script, and it
> has seen actions primarily from three people I Cc'ed this message to.
Thanks. Perhaps git-completion.bash should not say "Send all patches
to the current maintainer" and simply defer to SubmittingPatches?
>> contrib/completion/git-completion.bash | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 1505cff..8e7abb6 100755
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -738,6 +738,9 @@ __git_complete_remote_or_refspec ()
>> {
>> local cur_="$cur" cmd="${words[1]}"
>> local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
>> + if [ "$cmd" = "remote" ]; then
>> + c=$((++c))
>> + fi
>
> I don't know about others, but auto-incrementing a variable and assigning
> the result to the same variable, while not wrong at all, hurts my brain.
>
> c=$(($c + 1))
>
> is far more readable and does not suggest there is any funky magic
> involved. Also it is a good habit to get into not to omit $ from
> variables inside arithmetic substitution, even though bash allows it and
> this script is meant to be consumed only by shells that understand this
> bash-ism.
>
> I do not know offhand if zsh groks it, but the point is that you do not
> have to worry about it if you write "$(($c+1))" instead of "$((c+1))".
CodingGuidlines suggests to follow local convention, which was
"c=$((++c))". This file also uses "++n", "i++" and "((i++))". I will
send a v2 patch that normalizes these, open to discussion of course.
--
Philip Jägenstedt
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html