[PATCH v2 0/3] completion: bash: support for recursive/nested aliases

[PATCH v2 0/3] completion: bash: support for recursive/nested 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 v1:

 * Use a loop instead of recursive calls
 * Add a check to detect alias loops


Felipe Contreras (3):
  completion: bash: support recursive aliases
  completion: bash: check for alias loop
  completion: bash: simplify __git_aliased_command

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

-- 
2.29.2

[PATCH v2 1/3] completion: bash: support recursive aliases

[Felipe Contreras]

It is possible to have a 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 | 48 ++++++++++++++++----------
 t/t9902-completion.sh                  | 19 ++++++++++
 2 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 36f5a91c7a..59ced25641 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1121,26 +1121,36 @@ __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 [[ "$cur" != "$last" ]]; do
+		cmdline=$(__git config --get "alias.$cur")
+		last=$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
+
+	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 v2 2/3] 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 | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 59ced25641..bf2a59f95e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1121,11 +1121,17 @@ __git_pretty_aliases ()
 # __git_aliased_command requires 1 argument
 __git_aliased_command ()
 {
-	local cur=$1 last word cmdline
+	local cur=$1 last list word cmdline
 
 	while [[ "$cur" != "$last" ]]; do
+		if [[ "$list" == *"$cur "* ]]; then
+			# loop detected
+			return
+		fi
+
 		cmdline=$(__git config --get "alias.$cur")
 		last=$cur
+		list="$cur $list"
 
 		for word in $cmdline; do
 			case "$word" in
-- 
2.29.2

[PATCH v2 3/3] completion: bash: simplify __git_aliased_command

[Felipe Contreras]

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

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bf2a59f95e..ce0dc1e0f8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1121,17 +1121,17 @@ __git_pretty_aliases ()
 # __git_aliased_command requires 1 argument
 __git_aliased_command ()
 {
-	local cur=$1 last list word cmdline
+	local cur=$1 list word cmdline
 
-	while [[ "$cur" != "$last" ]]; do
+	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
 			case "$word" in
@@ -1154,6 +1154,7 @@ __git_aliased_command ()
 		done
 	done
 
+	cur="${list%% *}"
 	if [[ "$cur" != "$1" ]]; then
 		echo "$cur"
 	fi
-- 
2.29.2

Re: [PATCH v2 0/3] completion: bash: support for recursive/nested aliases

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Hello,
>
> As Philippe reported, aliases of aliases are not completed correctly in the
> Bash completion. This patch series attempts to fix that problem.
>
> Since v1:
>
>  * Use a loop instead of recursive calls
>  * Add a check to detect alias loops
>
>
> Felipe Contreras (3):
>   completion: bash: support recursive aliases
>   completion: bash: check for alias loop
>   completion: bash: simplify __git_aliased_command

It is unclear why 3/3 needs to be separate (in other words, is there
a reason why 1/3 and 2/3 need to be done while the function is in
the more complex form, instead of doing what 1/3 and 2/3 wanted to
do to the function in a way that does not require later clean-up?),
but other than that, the end-result looks good.

Will queue.

Re: [PATCH v2 0/3] completion: bash: support for recursive/nested aliases

[Felipe Contreras]

On Mon, Nov 9, 2020 at 7:47 PM Junio C Hamano <gitster@pobox.com> wrote:

> It is unclear why 3/3 needs to be separate (in other words, is there
> a reason why 1/3 and 2/3 need to be done while the function is in
> the more complex form, instead of doing what 1/3 and 2/3 wanted to
> do to the function in a way that does not require later clean-up?),
> but other than that, the end-result looks good.

It doesn't need to be separate. It's there just because the jump from
the current code to 1/3 is more natural this way (v2), but 3/3 can
certainly be merged to 1/3. We will have something a little bit odd in
the history:

  cur=$last
  if [[ "$cur" != "$1" ]]; then
    echo "$cur"
  fi

Why not just use $last instead of $cur? And in fact why clear $cur if
we are going to be setting it again?

It's just a little odd, but I sent a v3 with 1/3 and 3/3 merged, and
it's not too odd.

Cheers.

-- 
Felipe Contreras