Re: [PATCH 3/4] completion: fix branch.autosetup(merge|rebase)

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 2, 2014, 23:56 UTC
Message-ID: <xmqqa9fendj3.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1388415138-11011-4-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 9 quoted lines

>  	branch.*.*)
>  		local pfx="${cur%.*}." cur_="${cur##*.}"
> +		if [ "$pfx" == "branch.autosetupmerge." ] ||
> +			[ "$pfx" == "branch.autosetuprebase." ]; then
> +			return
> +		fi
>  		__gitcomp "remote pushremote merge mergeoptions rebase" "$pfx" "$cur_"
>  		return
>  		;;

I do not quite understand this change.

If we are looking at "branch.autosetupmerge." followed by something, who typed that final dot? If you are working on a topic about auto-setup-merge and named your branch "autosetupmerge", don't you want to be able to configure various aspect of that branch via branch.autosetupmerge.{remote,merge} etc., just like you can do so for your "topic" branch via branch.topic.{remote,merge} etc., regardless of your use of "autosetupmerge" option across branches?

Besides, it smells fishy to me that you need to enumerate and special case these two here, and then you have to repeat them below in a separate case arm.

Show 9 quoted lines

>  	branch.*)
>  		local pfx="${cur%.*}." cur_="${cur#*.}"
> -		__gitcomp_nl "$(__git_heads)" "$pfx" "$cur_" "."
> +		__gitcomp_2 "$(__git_heads)" "
> +			autosetupmerge autosetuprebase
> +			" "$pfx" "$cur_" "."
>  		return
>  		;;
>  	guitool.*.*)