git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH v2 0/3] completion: bash: support for recursive/nested aliases
@ 2020-11-10  0:53 Felipe Contreras
  2020-11-10  0:53 ` [PATCH v2 1/3] completion: bash: support recursive aliases Felipe Contreras
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Felipe Contreras @ 2020-11-10  0:53 UTC (permalink / raw)
  To: git
  Cc: Philippe Blain, Junio C Hamano, SZEDER Gábor, Jeff King,
	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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/3] completion: bash: support recursive aliases
  2020-11-10  0:53 [PATCH v2 0/3] completion: bash: support for recursive/nested aliases Felipe Contreras
@ 2020-11-10  0:53 ` Felipe Contreras
  2020-11-10  0:53 ` [PATCH v2 2/3] completion: bash: check for alias loop Felipe Contreras
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Felipe Contreras @ 2020-11-10  0:53 UTC (permalink / raw)
  To: git
  Cc: Philippe Blain, Junio C Hamano, SZEDER Gábor, Jeff King,
	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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 2/3] completion: bash: check for alias loop
  2020-11-10  0:53 [PATCH v2 0/3] completion: bash: support for recursive/nested aliases Felipe Contreras
  2020-11-10  0:53 ` [PATCH v2 1/3] completion: bash: support recursive aliases Felipe Contreras
@ 2020-11-10  0:53 ` Felipe Contreras
  2020-11-10  0:53 ` [PATCH v2 3/3] completion: bash: simplify __git_aliased_command Felipe Contreras
  2020-11-10  1:47 ` [PATCH v2 0/3] completion: bash: support for recursive/nested aliases Junio C Hamano
  3 siblings, 0 replies; 6+ messages in thread
From: Felipe Contreras @ 2020-11-10  0:53 UTC (permalink / raw)
  To: git
  Cc: Philippe Blain, Junio C Hamano, SZEDER Gábor, Jeff King,
	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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] completion: bash: simplify __git_aliased_command
  2020-11-10  0:53 [PATCH v2 0/3] completion: bash: support for recursive/nested aliases Felipe Contreras
  2020-11-10  0:53 ` [PATCH v2 1/3] completion: bash: support recursive aliases Felipe Contreras
  2020-11-10  0:53 ` [PATCH v2 2/3] completion: bash: check for alias loop Felipe Contreras
@ 2020-11-10  0:53 ` Felipe Contreras
  2020-11-10  1:47 ` [PATCH v2 0/3] completion: bash: support for recursive/nested aliases Junio C Hamano
  3 siblings, 0 replies; 6+ messages in thread
From: Felipe Contreras @ 2020-11-10  0:53 UTC (permalink / raw)
  To: git
  Cc: Philippe Blain, Junio C Hamano, SZEDER Gábor, Jeff King,
	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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 0/3] completion: bash: support for recursive/nested aliases
  2020-11-10  0:53 [PATCH v2 0/3] completion: bash: support for recursive/nested aliases Felipe Contreras
                   ` (2 preceding siblings ...)
  2020-11-10  0:53 ` [PATCH v2 3/3] completion: bash: simplify __git_aliased_command Felipe Contreras
@ 2020-11-10  1:47 ` Junio C Hamano
  2020-11-10  2:09   ` Felipe Contreras
  3 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2020-11-10  1:47 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: git, Philippe Blain, SZEDER Gábor, Jeff King

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 0/3] completion: bash: support for recursive/nested aliases
  2020-11-10  1:47 ` [PATCH v2 0/3] completion: bash: support for recursive/nested aliases Junio C Hamano
@ 2020-11-10  2:09   ` Felipe Contreras
  0 siblings, 0 replies; 6+ messages in thread
From: Felipe Contreras @ 2020-11-10  2:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git, Philippe Blain, SZEDER Gábor, Jeff King

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-11-10  2:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-10  0:53 [PATCH v2 0/3] completion: bash: support for recursive/nested aliases Felipe Contreras
2020-11-10  0:53 ` [PATCH v2 1/3] completion: bash: support recursive aliases Felipe Contreras
2020-11-10  0:53 ` [PATCH v2 2/3] completion: bash: check for alias loop Felipe Contreras
2020-11-10  0:53 ` [PATCH v2 3/3] completion: bash: simplify __git_aliased_command Felipe Contreras
2020-11-10  1:47 ` [PATCH v2 0/3] completion: bash: support for recursive/nested aliases Junio C Hamano
2020-11-10  2:09   ` Felipe Contreras

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).