Philip Jägenstedt <philip@xxxxxxxxxx> writes:
> Thanks. Perhaps git-completion.bash should not say "Send all patches
> to the current maintainer" and simply defer to SubmittingPatches?
I see you did this in your follow-up patch. Thanks.
>>> {
>>> 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.
OK, it was my mistake to suggest $c++ in this file; it liberally uses
dollar-less variables, and I agree that it is a good idea to stick to that
local convention.
But I think you went too far in your follow-up patch to make the increment
and decrement uniform.
"i++" is so much easier on the eye unless you must use "++i" in order to
use the value of the incremented "i" in an expression, and the changes to
turn existing instances of free-standing "i++" to "++i" done only for the
side effect of incrementing the variables look totally backwards.
Although I do not deeply care. Just leaving the new one as you originally
wrote, i.e.
c=$((++c))
would have been easier to review for the area experts, I would think.
--
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