From: SZEDER Gábor <szeder@ira.uka.de>
Date: Sat, 30 May 2015 19:01:03 GMT
Subject: Re: [PATCH v2 1/2] completion: Add sequencer function
Message-ID: <20150530210103.Horde.YoJthfdQJy5oq9wT_oOl3Q1@webmail.informatik.kit.edu>
In-Reply-To: <1433001710.5113.8.camel@virtuell-zuhause.de>


Quoting Thomas Braun <thomas.braun@virtuell-zuhause.de>:

> Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de>
> ---
>  contrib/completion/git-completion.bash | 48  
> +++++++++++++++++++++++-----------
>  1 file changed, 33 insertions(+), 15 deletions(-)

I don't see the benefits of this change.  This patch adds more than  
twice as many lines as it removes, and patch 2/2 adds 8 new lines  
although it could get away with only 5 without this function.  To  
offer sequencer options we currently go through a single if statement,  
with this patch we'd go through a case statement, an if statement and  
finally an &&.

Gábor


> diff --git a/contrib/completion/git-completion.bash  
> b/contrib/completion/git-completion.bash
> index bfc74e9..f6e5bf6 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -851,15 +851,40 @@ __git_count_arguments ()
>  	printf "%d" $c
>  }
>
> +__git_complete_sequencer ()
> +{
> +	local dir="$(__gitdir)"
> +
> +	case "$1" in
> +	am)
> +		if [ -d "$dir"/rebase-apply ]; then
> +			__gitcomp "--skip --continue --resolved --abort"
> +			return 0
> +		fi
> +		;;
> +	cherry-pick)
> +		if [ -f "$dir"/CHERRY_PICK_HEAD ]; then
> +			__gitcomp "--continue --quit --abort"
> +			return 0
> +		fi
> +		;;
> +	rebase)
> +		if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
> +			__gitcomp "--continue --skip --abort"
> +			return 0
> +		fi
> +		;;
> +	esac
> +
> +	return 1
> +}
> +
>  __git_whitespacelist="nowarn warn error error-all fix"
>
>  _git_am ()
>  {
> -	local dir="$(__gitdir)"
> -	if [ -d "$dir"/rebase-apply ]; then
> -		__gitcomp "--skip --continue --resolved --abort"
> -		return
> -	fi
> +	__git_complete_sequencer "am" && return
> +
>  	case "$cur" in
>  	--whitespace=*)
>  		__gitcomp "$__git_whitespacelist" "" "${cur##--whitespace=}"
> @@ -1044,11 +1069,8 @@ _git_cherry ()
>
>  _git_cherry_pick ()
>  {
> -	local dir="$(__gitdir)"
> -	if [ -f "$dir"/CHERRY_PICK_HEAD ]; then
> -		__gitcomp "--continue --quit --abort"
> -		return
> -	fi
> +	__git_complete_sequencer "cherry-pick" && return
> +
>  	case "$cur" in
>  	--*)
>  		__gitcomp "--edit --no-commit --signoff --strategy= --mainline"
> @@ -1666,11 +1688,7 @@ _git_push ()
>
>  _git_rebase ()
>  {
> -	local dir="$(__gitdir)"
> -	if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
> -		__gitcomp "--continue --skip --abort"
> -		return
> -	fi
> +	__git_complete_sequencer "rebase" && return
>  	__git_complete_strategy && return
>  	case "$cur" in
>  	--whitespace=*)