Re: [PATCH] completion: do not cache if --git-completion-helper fails

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 7, 2019, 16:35 UTC
Message-ID: <xmqqzhmtpd44.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190607093034.816-1-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 10 quoted lines

>  	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.