Re: [PATCH 08/19] completion: use $__git_dir instead of $(__gitdir)
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 9, 2012, 20:56 UTC
- Message-ID
- <7vmx5hqdjx.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20120509202220.GB7824@goldbirke>
SZEDER Gábor <szeder@ira.uka.de> writes:
Show 22 quoted lines
>> > @@ -962,7 +967,8 @@ __git_aliases ()
>> > # __git_aliased_command requires 1 argument
>> > __git_aliased_command ()
>> > {
>> > - local word cmdline=$(git --git-dir="$(__gitdir)" \
>> > + __gitdir >/dev/null
>> > + local word cmdline=$(git --git-dir="$__git_dir" \
>> > config --get "alias.$1")
>> > for word in $cmdline; do
>> > case "$word" in
>>
>> Now this worries me. The way I read 07/19 was that the local __git_dir=""
>> declarations in __git_ps1 and __git were what protected this whole
>> machinery to protect us against surprises from user doing "cd" between
>> interactive commands, but you have the same __gitdir call to set up the
>> global $__git_dir variable there, without the initialization to "".
>>
>> Having to have a call to __gitdir seems to indicate to me that you cannot
>> assume that the other initialization sites may not have been called before
>> we get to this point. Then why is 'local __git_dir=""' unneeded here?
>
> Your comments to the previous patch apply here.Not really. __git_ps1 and __gitk seems to do __gitdir very early to make sure anybody that use $__git_dir can rely on it, but having to sprinkle "set up $__git_dir variable" everywhere means anybody who wants to update need to know if it is already called, which defeats the point of "we can use $__git_dir instead of calling $(__gitdir)" from maintainability's point of view.