From: Junio C Hamano Date: Wed, 09 May 2012 20:56:02 GMT Subject: Re: [PATCH 08/19] completion: use $__git_dir instead of $(__gitdir) Message-ID: <7vmx5hqdjx.fsf@alter.siamese.dyndns.org> In-Reply-To: <20120509202220.GB7824@goldbirke> SZEDER Gábor writes: >> > @@ -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.