From: Junio C Hamano Date: Fri, 07 Jun 2019 16:35:39 GMT Subject: Re: [PATCH] completion: do not cache if --git-completion-helper fails Message-ID: In-Reply-To: <20190607093034.816-1-pclouds@gmail.com> Nguyễn Thái Ngọc Duy writes: > if [ -z "$options" ]; then > + local nocache= > # leading and trailing spaces are significant to make > # option removal work correctly. > - options=" $incl $(__git ${cmd/_/ } --git-completion-helper) " > + options=" $incl $(__git ${cmd/_/ } --git-completion-helper) " || nocache=t > + > for i in $excl; do > options="${options/ $i / }" > done Is there a point in doing this loop if we are not going to eval after all? IOW... > - eval "$var=\"$options\"" > + test -n "$nocache" || eval "$var=\"$options\"" > fi ... I am wondering why it is not more like this if options=" $incl $(__git ${cmd/_/ } --git-completion-helper) " then for i in $excl do options=... done eval "$var=..." fi or just return from the function instead of introducing and setting a new variable, as only remaining thing the function does is to call __gitcomp with $options, but we know that we are giving up on completing this round.