[PATCH v3 0/2] completion: bash: support for recursive aliases

[PATCH v3 0/2] completion: bash: support for recursive aliases

[Felipe Contreras]

Hello,

As Philippe reported, aliases of aliases are not completed correctly in the
Bash completion. This patch series attempts to fix that problem.

Since v2:

 * Reshuffle the commits to have a simplified history


Felipe Contreras (2):
  completion: bash: support recursive aliases
  completion: bash: check for alias loop

 contrib/completion/git-completion.bash | 53 +++++++++++++++++---------
 t/t9902-completion.sh                  | 19 +++++++++
 2 files changed, 54 insertions(+), 18 deletions(-)

-- 
2.29.2

[PATCH v3 1/2] completion: bash: support recursive aliases

[Felipe Contreras]

It is possible to have recursive aliases like:

  l = log --oneline
  lg = l --graph

So the completion should detect such aliases as well.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 50 ++++++++++++++++----------
 t/t9902-completion.sh                  | 19 ++++++++++
 2 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 36f5a91c7a..eb00d9ba22 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1121,26 +1121,38 @@ __git_pretty_aliases ()
 # __git_aliased_command requires 1 argument
 __git_aliased_command ()
 {
-	local word cmdline=$(__git config --get "alias.$1")
-	for word in $cmdline; do
-		case "$word" in
-		\!gitk|gitk)
-			echo "gitk"
-			return
-			;;
-		\!*)	: shell command alias ;;
-		-*)	: option ;;
-		*=*)	: setting env ;;
-		git)	: git itself ;;
-		\(\))   : skip parens of shell function definition ;;
-		{)	: skip start of shell helper function ;;
-		:)	: skip null command ;;
-		\'*)	: skip opening quote after sh -c ;;
-		*)
-			echo "$word"
-			return
-		esac
+	local cur=$1 last word cmdline
+
+	while [[ -n "$cur" ]]; do
+		cmdline=$(__git config --get "alias.$cur")
+		last=$cur
+		cur=
+
+		for word in $cmdline; do
+			case "$word" in
+			\!gitk|gitk)
+				cur="gitk"
+				break
+				;;
+			\!*)	: shell command alias ;;
+			-*)	: option ;;
+			*=*)	: setting env ;;
+			git)	: git itself ;;
+			\(\))   : skip parens of shell function definition ;;
+			{)	: skip start of shell helper function ;;
+			:)	: skip null command ;;
+			\'*)	: skip opening quote after sh -c ;;
+			*)
+				cur="$word"
+				break
+			esac
+		done
 	done
+
+	cur=$last
+	if [[ "$cur" != "$1" ]]; then
+		echo "$cur"
+	fi
 }
 
 # Check whether one of the given words is present on the command line,
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 4e943393cf..0e2db6e7fa 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2195,6 +2195,25 @@ test_expect_success 'complete files' '
 	test_completion "git add mom" "momified"
 '
 
+test_expect_success "simple alias" '
+	test_config alias.co checkout &&
+	test_completion "git co m" <<-\EOF
+	master Z
+	mybranch Z
+	mytag Z
+	EOF
+'
+
+test_expect_success "recursive alias" '
+	test_config alias.co checkout &&
+	test_config alias.cod "co --detached" &&
+	test_completion "git cod m" <<-\EOF
+	master Z
+	mybranch Z
+	mytag Z
+	EOF
+'
+
 test_expect_success "completion uses <cmd> completion for alias: !sh -c 'git <cmd> ...'" '
 	test_config alias.co "!sh -c '"'"'git checkout ...'"'"'" &&
 	test_completion "git co m" <<-\EOF
-- 
2.29.2

[PATCH v3 2/2] completion: bash: check for alias loop

[Felipe Contreras]

We don't want to be stuck in an endless cycle.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index eb00d9ba22..ce0dc1e0f8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1121,11 +1121,16 @@ __git_pretty_aliases ()
 # __git_aliased_command requires 1 argument
 __git_aliased_command ()
 {
-	local cur=$1 last word cmdline
+	local cur=$1 list word cmdline
 
 	while [[ -n "$cur" ]]; do
+		if [[ "$list" == *"$cur "* ]]; then
+			# loop detected
+			return
+		fi
+
 		cmdline=$(__git config --get "alias.$cur")
-		last=$cur
+		list="$cur $list"
 		cur=
 
 		for word in $cmdline; do
@@ -1149,7 +1154,7 @@ __git_aliased_command ()
 		done
 	done
 
-	cur=$last
+	cur="${list%% *}"
 	if [[ "$cur" != "$1" ]]; then
 		echo "$cur"
 	fi
-- 
2.29.2

Re: [PATCH v3 2/2] completion: bash: check for alias loop

[SZEDER Gábor]

On Mon, Nov 09, 2020 at 08:03:43PM -0600, Felipe Contreras wrote:
> We don't want to be stuck in an endless cycle.
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index eb00d9ba22..ce0dc1e0f8 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1121,11 +1121,16 @@ __git_pretty_aliases ()
>  # __git_aliased_command requires 1 argument
>  __git_aliased_command ()
>  {
> -	local cur=$1 last word cmdline
> +	local cur=$1 list word cmdline
>  
>  	while [[ -n "$cur" ]]; do
> +		if [[ "$list" == *"$cur "* ]]; then

I suspect the right hand side should be *" $cur "* ...

> +			# loop detected
> +			return
> +		fi
> +
>  		cmdline=$(__git config --get "alias.$cur")
> -		last=$cur
> +		list="$cur $list"

... and this should be list=" $cur $list", because otherwise a partial
match is possible and would be mistaken for a loop (though I didn't
actually test whether that's indeed the case).

>  		cur=
>  
>  		for word in $cmdline; do
> @@ -1149,7 +1154,7 @@ __git_aliased_command ()
>  		done
>  	done
>  
> -	cur=$last
> +	cur="${list%% *}"
>  	if [[ "$cur" != "$1" ]]; then
>  		echo "$cur"
>  	fi
> -- 
> 2.29.2
> 

Re: [PATCH v3 2/2] completion: bash: check for alias loop

[Junio C Hamano]

SZEDER Gábor <szeder.dev@gmail.com> writes:

>>  	while [[ -n "$cur" ]]; do
>> +		if [[ "$list" == *"$cur "* ]]; then
>
> I suspect the right hand side should be *" $cur "* ...
>
>> +			# loop detected
>> +			return
>> +		fi
>> +
>>  		cmdline=$(__git config --get "alias.$cur")
>> -		last=$cur
>> +		list="$cur $list"
>
> ... and this should be list=" $cur $list", because otherwise a partial
> match is possible and would be mistaken for a loop (though I didn't
> actually test whether that's indeed the case).

Traditionist may use

	list=
	while :
	do
		cur=$(obtain cur somehow)
                case " $list " in
                *" $cur "*) : cur appears in list ;;
                esac
                list="$cur${list+ }$list"
	done

to make the invariant "$list is a SP-separated tokens, no extra SP on
either ends" and I think the same idea would work with [[ $a == $b ]],
but "one SP on both ends, two SPs in between" like yours do is OK, too.

Not keeping excess SPs out of the list (unlike the above "no extra
SP on ends") means how "finally, take the first token in the list,
that's the answer" is coded, though.  You'd need to split off the
leading SP from it (below).

>>  		cur=
>>  
>>  		for word in $cmdline; do
>> @@ -1149,7 +1154,7 @@ __git_aliased_command ()
>>  		done
>>  	done
>>  
>> -	cur=$last
>> +	cur="${list%% *}"
>>  	if [[ "$cur" != "$1" ]]; then
>>  		echo "$cur"
>>  	fi
>> -- 
>> 2.29.2
>>