All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] improve bash completion of fetch, pull, and push
@ 2009-03-06  4:39 Jay Soffian
  2009-03-06  4:39 ` [PATCH 1/3] bash completion: fix completion issues with " Jay Soffian
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Jay Soffian @ 2009-03-06  4:39 UTC (permalink / raw)
  To: git
  Cc: Jay Soffian, Markus Heidelberg, Sverre Rabbelier, Junio C Hamano,
	Shawn O . Pearce

On Thu, Mar 5, 2009 at 6:15 PM, Markus Heidelberg <markus.heidelberg@web.de> wrote:
> Sverre Rabbelier, 05.03.2009:
>> Heya,
>>
>> Observe:
>> $ git push ori<tab>
>>   git push origin
>>
>> $ git push -f ori<tab>
>>   git push -f origin/
>>
>> Something weird going on there, or is this intentional and am I
>> missing something?
>
> Something similar happens with fetch and pull. They only complete the
> remote name, when exactly 2 words are existing on the command line
> ("git" and the subcommand) by: if [ "$COMP_CWORD" = 2 ]
>
> Doesn't seem right.

This series is intended to fix the original issue, as well as provide
--option completion for all three commands. And, I made a clean spot, so
I had to clean up a couple other things.

Jay Soffian (3):
  bash completion: fix completion issues with fetch, pull, and push
  bash completion: refactor --strategy completion
  bash completion: teach fetch, pull, and push to complete their
    options

 contrib/completion/git-completion.bash |  197 +++++++++++++++++++++-----------
 1 files changed, 130 insertions(+), 67 deletions(-)

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

* [PATCH 1/3] bash completion: fix completion issues with fetch, pull, and push
  2009-03-06  4:39 [PATCH 0/3] improve bash completion of fetch, pull, and push Jay Soffian
@ 2009-03-06  4:39 ` Jay Soffian
  2009-03-06 15:58   ` Shawn O. Pearce
  2009-03-06  4:39 ` [PATCH 2/3] bash completion: refactor --strategy completion Jay Soffian
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Jay Soffian @ 2009-03-06  4:39 UTC (permalink / raw)
  To: git
  Cc: Jay Soffian, Markus Heidelberg, Sverre Rabbelier, Junio C Hamano,
	Shawn O . Pearce

Sverre Rabbelier noticed a completion issue with push:

 $ git push ori<tab>
 git push origin

 $ git push -f ori<tab>
 git push -f origin/

Markus Heidelberg pointed out that the issue extends to fetch and pull.

The reason is that the current code naively assumes that if
COMP_CWORD=2, it should complete a remote name, otherwise it should
complete a refspec. This assumption fails if there are any --options.

This patch fixes that issue by instead scanning COMP_CWORDS to see if
the remote has been completed yet (we now assume the first non-dashed
argument is the remote). The new logic is factored into a function,
shared by fetch, pull, and push.

The new function also properly handles '.' as the remote.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 contrib/completion/git-completion.bash |  109 +++++++++++++++++--------------
 1 files changed, 60 insertions(+), 49 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0a3092f..b347fdd 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -383,6 +383,63 @@ __git_complete_revlist ()
 	esac
 }
 
+__git_complete_remote_or_refspec ()
+{
+	local cmd="${COMP_WORDS[1]}"
+	local cur="${COMP_WORDS[COMP_CWORD]}"
+	local i c=2 remote="" pfx="" lhs=1
+	while [ $c -lt $COMP_CWORD ]; do
+		i="${COMP_WORDS[c]}"
+		case "$i" in
+		-*) ;;
+		*) remote="$i"; break ;;
+		esac
+		c=$((++c))
+	done
+	if [ -z "$remote" ]; then
+		__gitcomp "$(__git_remotes)"
+		return
+	fi
+	[ "$remote" = "." ] && remote=
+	case "$cur" in
+	*:*)
+		case "$COMP_WORDBREAKS" in
+		*:*) : great ;;
+		*)   pfx="${cur%%:*}:" ;;
+		esac
+		cur="${cur#*:}"
+		lhs=0
+		;;
+	+*)
+		pfx="+"
+		cur="${cur#+}"
+		;;
+	esac
+	case "$cmd" in
+	fetch)
+		if [ $lhs = 1 ]; then
+			__gitcomp "$(__git_refs2 "$remote")" "$pfx" "$cur"
+		else
+			__gitcomp "$(__git_refs)" "$pfx" "$cur"
+		fi
+		;;
+	pull)
+		if [ $lhs = 1 ]; then
+			__gitcomp "$(__git_refs "$remote")" "$pfx" "$cur"
+		else
+			__gitcomp "$(__git_refs)" "$pfx" "$cur"
+		fi
+		;;
+	push)
+		if [ $lhs = 1 ]; then
+			__gitcomp "$(__git_refs)" "$pfx" "$cur"
+		else
+			__gitcomp "$(__git_refs "$remote")" "$pfx" "$cur"
+		fi
+		;;
+	esac
+}
+
 __git_all_commands ()
 {
 	if [ -n "$__git_all_commandlist" ]; then
@@ -828,25 +885,7 @@ _git_diff ()
 
 _git_fetch ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
-
-	if [ "$COMP_CWORD" = 2 ]; then
-		__gitcomp "$(__git_remotes)"
-	else
-		case "$cur" in
-		*:*)
-			local pfx=""
-			case "$COMP_WORDBREAKS" in
-			*:*) : great ;;
-			*)   pfx="${cur%%:*}:" ;;
-			esac
-			__gitcomp "$(__git_refs)" "$pfx" "${cur#*:}"
-			;;
-		*)
-			__gitcomp "$(__git_refs2 "${COMP_WORDS[2]}")"
-			;;
-		esac
-	fi
+	__git_complete_remote_or_refspec
 }
 
 _git_format_patch ()
@@ -1111,40 +1150,12 @@ _git_name_rev ()
 
 _git_pull ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
-
-	if [ "$COMP_CWORD" = 2 ]; then
-		__gitcomp "$(__git_remotes)"
-	else
-		__gitcomp "$(__git_refs "${COMP_WORDS[2]}")"
-	fi
+	__git_complete_remote_or_refspec
 }
 
 _git_push ()
 {
-	local cur="${COMP_WORDS[COMP_CWORD]}"
-
-	if [ "$COMP_CWORD" = 2 ]; then
-		__gitcomp "$(__git_remotes)"
-	else
-		case "$cur" in
-		*:*)
-			local pfx=""
-			case "$COMP_WORDBREAKS" in
-			*:*) : great ;;
-			*)   pfx="${cur%%:*}:" ;;
-			esac
-
-			__gitcomp "$(__git_refs "${COMP_WORDS[2]}")" "$pfx" "${cur#*:}"
-			;;
-		+*)
-			__gitcomp "$(__git_refs)" + "${cur#+}"
-			;;
-		*)
-			__gitcomp "$(__git_refs)"
-			;;
-		esac
-	fi
+	__git_complete_remote_or_refspec
 }
 
 _git_rebase ()
-- 
1.6.2.rc2.332.g5d21b

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

* [PATCH 2/3] bash completion: refactor --strategy completion
  2009-03-06  4:39 [PATCH 0/3] improve bash completion of fetch, pull, and push Jay Soffian
  2009-03-06  4:39 ` [PATCH 1/3] bash completion: fix completion issues with " Jay Soffian
@ 2009-03-06  4:39 ` Jay Soffian
  2009-03-06 16:04   ` Shawn O. Pearce
  2009-03-06  4:39 ` [PATCH 3/3] bash completion: teach fetch, pull, and push to complete their options Jay Soffian
  2009-03-06  6:43 ` [PATCH 0/3] improve bash completion of fetch, pull, and push Sverre Rabbelier
  3 siblings, 1 reply; 14+ messages in thread
From: Jay Soffian @ 2009-03-06  4:39 UTC (permalink / raw)
  To: git
  Cc: Jay Soffian, Markus Heidelberg, Sverre Rabbelier, Junio C Hamano,
	Shawn O . Pearce

The code to complete --strategy was duplicated between _git_rebase and
_git_merge, and is about to gain a third caller (_git_pull). This patch
factors it into its own function.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 contrib/completion/git-completion.bash |   37 ++++++++++++++++---------------
 1 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b347fdd..8924185 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -440,6 +440,22 @@ __git_complete_remote_or_refspec ()
 	esac
 }
 
+__git_complete_strategy ()
+{
+	case "${COMP_WORDS[COMP_CWORD-1]}" in
+	-s|--strategy)
+		__gitcomp "$(__git_merge_strategies)"
+		return 1
+	esac
+	case "$cur" in
+	--strategy=*)
+		__gitcomp "$(__git_merge_strategies)" "" "${cur##--strategy=}"
+		return 1
+		;;
+	esac
+	return 0
+}
+
 __git_all_commands ()
 {
 	if [ -n "$__git_all_commandlist" ]; then
@@ -1086,17 +1102,10 @@ _git_log ()
 
 _git_merge ()
 {
+	__git_complete_strategy && return
+
 	local cur="${COMP_WORDS[COMP_CWORD]}"
-	case "${COMP_WORDS[COMP_CWORD-1]}" in
-	-s|--strategy)
-		__gitcomp "$(__git_merge_strategies)"
-		return
-	esac
 	case "$cur" in
-	--strategy=*)
-		__gitcomp "$(__git_merge_strategies)" "" "${cur##--strategy=}"
-		return
-		;;
 	--*)
 		__gitcomp "
 			--no-commit --no-stat --log --no-log --squash --strategy
@@ -1165,16 +1174,8 @@ _git_rebase ()
 		__gitcomp "--continue --skip --abort"
 		return
 	fi
-	case "${COMP_WORDS[COMP_CWORD-1]}" in
-	-s|--strategy)
-		__gitcomp "$(__git_merge_strategies)"
-		return
-	esac
+	__git_complete_strategy && return
 	case "$cur" in
-	--strategy=*)
-		__gitcomp "$(__git_merge_strategies)" "" "${cur##--strategy=}"
-		return
-		;;
 	--*)
 		__gitcomp "--onto --merge --strategy --interactive"
 		return
-- 
1.6.2.rc2.332.g5d21b

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

* [PATCH 3/3] bash completion: teach fetch, pull, and push to complete their options
  2009-03-06  4:39 [PATCH 0/3] improve bash completion of fetch, pull, and push Jay Soffian
  2009-03-06  4:39 ` [PATCH 1/3] bash completion: fix completion issues with " Jay Soffian
  2009-03-06  4:39 ` [PATCH 2/3] bash completion: refactor --strategy completion Jay Soffian
@ 2009-03-06  4:39 ` Jay Soffian
  2009-03-06 16:15   ` Shawn O. Pearce
  2009-03-06  6:43 ` [PATCH 0/3] improve bash completion of fetch, pull, and push Sverre Rabbelier
  3 siblings, 1 reply; 14+ messages in thread
From: Jay Soffian @ 2009-03-06  4:39 UTC (permalink / raw)
  To: git
  Cc: Jay Soffian, Markus Heidelberg, Sverre Rabbelier, Junio C Hamano,
	Shawn O . Pearce

fetch, pull, and push didn't know their options. They do now. merge's
options are factored into a variable so they can be shared between
_git_merge and _git_pull

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 contrib/completion/git-completion.bash |   61 +++++++++++++++++++++++++++++---
 1 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8924185..3ebedea 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -387,10 +387,11 @@ __git_complete_remote_or_refspec ()
 {
 	local cmd="${COMP_WORDS[1]}"
 	local cur="${COMP_WORDS[COMP_CWORD]}"
-	local i c=2 remote="" pfx="" lhs=1
+	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
 	while [ $c -lt $COMP_CWORD ]; do
 		i="${COMP_WORDS[c]}"
 		case "$i" in
+		--all|--mirror) [ "$cmd" = "push" ] && no_complete_refspec=1 ;;
 		-*) ;;
 		*) remote="$i"; break ;;
 		esac
@@ -400,6 +401,10 @@ __git_complete_remote_or_refspec ()
 		__gitcomp "$(__git_remotes)"
 		return
 	fi
+	if [ $no_complete_refspec = 1 ]; then
+		COMPREPLY=()
+		return
+	fi
 	[ "$remote" = "." ] && remote=
 	case "$cur" in
 	*:*)
@@ -899,8 +904,20 @@ _git_diff ()
 	__git_complete_file
 }
 
+__git_fetch_options="
+	--quiet --verbose --append --upload-pack --force --keep --depth=
+	--tags --no-tags
+"
+
 _git_fetch ()
 {
+	local cur="${COMP_WORDS[COMP_CWORD]}"
+	case "$cur" in
+	--*)
+		__gitcomp "$__git_fetch_options"
+		return
+		;;
+	esac
 	__git_complete_remote_or_refspec
 }
 
@@ -1100,6 +1117,11 @@ _git_log ()
 	__git_complete_revlist
 }
 
+__git_merge_options="
+	--no-commit --no-stat --log --no-log --squash --strategy
+	--commit --stat --no-squash --ff --no-ff
+"
+
 _git_merge ()
 {
 	__git_complete_strategy && return
@@ -1107,10 +1129,7 @@ _git_merge ()
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 	case "$cur" in
 	--*)
-		__gitcomp "
-			--no-commit --no-stat --log --no-log --squash --strategy
-			--commit --stat --no-squash --ff --no-ff
-			"
+		__gitcomp "$__git_merge_options"
 		return
 	esac
 	__gitcomp "$(__git_refs)"
@@ -1159,11 +1178,43 @@ _git_name_rev ()
 
 _git_pull ()
 {
+	__git_complete_strategy && return
+
+	local cur="${COMP_WORDS[COMP_CWORD]}"
+	case "$cur" in
+	--*)
+		__gitcomp "
+			--rebase --no-rebase
+			$__git_merge_options
+			$__git_fetch_options
+		"
+		return
+		;;
+	esac
 	__git_complete_remote_or_refspec
 }
 
 _git_push ()
 {
+	local cur="${COMP_WORDS[COMP_CWORD]}"
+	case "${COMP_WORDS[COMP_CWORD-1]}" in
+	--repo)
+		__gitcomp "$(__git_remotes)"
+		return
+	esac
+	case "$cur" in
+	--repo=*)
+		__gitcomp "$(__git_remotes)" "" "${cur##--repo=}"
+		return
+		;;
+	--*)
+		__gitcomp "
+			--all --mirror --tags --dry-run --force --verbose
+			--receive-pack= --repo=
+		"
+		return
+		;;
+	esac
 	__git_complete_remote_or_refspec
 }
 
-- 
1.6.2.rc2.332.g5d21b

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

* Re: [PATCH 0/3] improve bash completion of fetch, pull, and push
  2009-03-06  4:39 [PATCH 0/3] improve bash completion of fetch, pull, and push Jay Soffian
                   ` (2 preceding siblings ...)
  2009-03-06  4:39 ` [PATCH 3/3] bash completion: teach fetch, pull, and push to complete their options Jay Soffian
@ 2009-03-06  6:43 ` Sverre Rabbelier
  3 siblings, 0 replies; 14+ messages in thread
From: Sverre Rabbelier @ 2009-03-06  6:43 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, Markus Heidelberg, Junio C Hamano, Shawn O . Pearce

Heya,

On Fri, Mar 6, 2009 at 05:39, Jay Soffian <jaysoffian@gmail.com> wrote:
> This series is intended to fix the original issue, as well as provide
> --option completion for all three commands. And, I made a clean spot, so
> I had to clean up a couple other things.

Thanks, I can't comment on the quality of the patch, but I'm glad you
went through the effort of fixing this :).

-- 
Cheers,

Sverre Rabbelier

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

* Re: [PATCH 1/3] bash completion: fix completion issues with fetch, pull, and push
  2009-03-06  4:39 ` [PATCH 1/3] bash completion: fix completion issues with " Jay Soffian
@ 2009-03-06 15:58   ` Shawn O. Pearce
  2009-03-11 10:02     ` Nanako Shiraishi
  0 siblings, 1 reply; 14+ messages in thread
From: Shawn O. Pearce @ 2009-03-06 15:58 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, Markus Heidelberg, Sverre Rabbelier, Junio C Hamano

Jay Soffian <jaysoffian@gmail.com> wrote:
> Sverre Rabbelier noticed a completion issue with push:
> 
>  $ git push ori<tab>
>  git push origin
> 
>  $ git push -f ori<tab>
>  git push -f origin/
> 
> Markus Heidelberg pointed out that the issue extends to fetch and pull.
> 
> The reason is that the current code naively assumes that if
> COMP_CWORD=2, it should complete a remote name, otherwise it should
> complete a refspec. This assumption fails if there are any --options.
> 
> This patch fixes that issue by instead scanning COMP_CWORDS to see if
> the remote has been completed yet (we now assume the first non-dashed
> argument is the remote). The new logic is factored into a function,
> shared by fetch, pull, and push.
> 
> The new function also properly handles '.' as the remote.
> 
> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>

Acked-by: Shawn O. Pearce <spearce@spearce.org>


> ---
>  contrib/completion/git-completion.bash |  109 +++++++++++++++++--------------
>  1 files changed, 60 insertions(+), 49 deletions(-)

-- 
Shawn.

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

* Re: [PATCH 2/3] bash completion: refactor --strategy completion
  2009-03-06  4:39 ` [PATCH 2/3] bash completion: refactor --strategy completion Jay Soffian
@ 2009-03-06 16:04   ` Shawn O. Pearce
  2009-03-06 16:12     ` Jay Soffian
  0 siblings, 1 reply; 14+ messages in thread
From: Shawn O. Pearce @ 2009-03-06 16:04 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, Markus Heidelberg, Sverre Rabbelier, Junio C Hamano

Jay Soffian <jaysoffian@gmail.com> wrote:
> The code to complete --strategy was duplicated between _git_rebase and
> _git_merge, and is about to gain a third caller (_git_pull). This patch
> factors it into its own function.
> 
> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>

Looks OK to me, but I had to squash this in to get it to work.
Without this the patch breaks git merge completion entirely:

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9e16576..80d464b 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -449,15 +449,15 @@ __git_complete_strategy ()
 	case "${COMP_WORDS[COMP_CWORD-1]}" in
 	-s|--strategy)
 		__gitcomp "$(__git_merge_strategies)"
-		return 1
+		return 0
 	esac
 	case "$cur" in
 	--strategy=*)
 		__gitcomp "$(__git_merge_strategies)" "" "${cur##--strategy=}"
-		return 1
+		return 0
 		;;
 	esac
-	return 0
+	return 1
 }
 
 __git_all_commands ()
@@ -1111,9 +1111,8 @@ _git_log ()
 
 _git_merge ()
 {
-	__git_complete_strategy && return
-
 	local cur="${COMP_WORDS[COMP_CWORD]}"
+	__git_complete_strategy && return
 	case "$cur" in
 	--*)
 		__gitcomp "

> ---
>  contrib/completion/git-completion.bash |   37 ++++++++++++++++---------------
>  1 files changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index b347fdd..8924185 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -440,6 +440,22 @@ __git_complete_remote_or_refspec ()
>  	esac
>  }
>  
> +__git_complete_strategy ()
> +{
> +	case "${COMP_WORDS[COMP_CWORD-1]}" in
> +	-s|--strategy)
> +		__gitcomp "$(__git_merge_strategies)"
> +		return 1
> +	esac
> +	case "$cur" in
> +	--strategy=*)
> +		__gitcomp "$(__git_merge_strategies)" "" "${cur##--strategy=}"
> +		return 1
> +		;;
> +	esac
> +	return 0
> +}
> +
>  __git_all_commands ()
>  {
>  	if [ -n "$__git_all_commandlist" ]; then
> @@ -1086,17 +1102,10 @@ _git_log ()
>  
>  _git_merge ()
>  {
> +	__git_complete_strategy && return
> +
>  	local cur="${COMP_WORDS[COMP_CWORD]}"
> -	case "${COMP_WORDS[COMP_CWORD-1]}" in
> -	-s|--strategy)
> -		__gitcomp "$(__git_merge_strategies)"
> -		return
> -	esac
>  	case "$cur" in
> -	--strategy=*)
> -		__gitcomp "$(__git_merge_strategies)" "" "${cur##--strategy=}"
> -		return
> -		;;
>  	--*)
>  		__gitcomp "
>  			--no-commit --no-stat --log --no-log --squash --strategy
> @@ -1165,16 +1174,8 @@ _git_rebase ()
>  		__gitcomp "--continue --skip --abort"
>  		return
>  	fi
> -	case "${COMP_WORDS[COMP_CWORD-1]}" in
> -	-s|--strategy)
> -		__gitcomp "$(__git_merge_strategies)"
> -		return
> -	esac
> +	__git_complete_strategy && return
>  	case "$cur" in
> -	--strategy=*)
> -		__gitcomp "$(__git_merge_strategies)" "" "${cur##--strategy=}"
> -		return
> -		;;
>  	--*)
>  		__gitcomp "--onto --merge --strategy --interactive"
>  		return
> -- 
> 1.6.2.rc2.332.g5d21b
> 

-- 
Shawn.

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

* Re: [PATCH 2/3] bash completion: refactor --strategy completion
  2009-03-06 16:04   ` Shawn O. Pearce
@ 2009-03-06 16:12     ` Jay Soffian
  2009-03-06 16:16       ` Shawn O. Pearce
  0 siblings, 1 reply; 14+ messages in thread
From: Jay Soffian @ 2009-03-06 16:12 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Markus Heidelberg, Sverre Rabbelier, Junio C Hamano

On Fri, Mar 6, 2009 at 11:04 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Jay Soffian <jaysoffian@gmail.com> wrote:
>> The code to complete --strategy was duplicated between _git_rebase and
>> _git_merge, and is about to gain a third caller (_git_pull). This patch
>> factors it into its own function.
>>
>> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
>
> Looks OK to me, but I had to squash this in to get it to work.
> Without this the patch breaks git merge completion entirely:

Weird, I had it completely backwards but it seemed to work. Probably
because I was sourcing the new completion into my running shell; I'll
make sure to startup a new shell next time.

With the squash (assuming Junio doesn't mind), is it acked-by you or
should I resend?

j.

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

* Re: [PATCH 3/3] bash completion: teach fetch, pull, and push to complete their options
  2009-03-06  4:39 ` [PATCH 3/3] bash completion: teach fetch, pull, and push to complete their options Jay Soffian
@ 2009-03-06 16:15   ` Shawn O. Pearce
  0 siblings, 0 replies; 14+ messages in thread
From: Shawn O. Pearce @ 2009-03-06 16:15 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, Markus Heidelberg, Sverre Rabbelier, Junio C Hamano

Jay Soffian <jaysoffian@gmail.com> wrote:
> fetch, pull, and push didn't know their options. They do now. merge's
> options are factored into a variable so they can be shared between
> _git_merge and _git_pull
> 
> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>

Acked-by: Shawn O. Pearce <spearce@spearce.org>

but I think this patch depends on 2/3 in the series to be fixed first.

> ---
>  contrib/completion/git-completion.bash |   61 +++++++++++++++++++++++++++++---
>  1 files changed, 56 insertions(+), 5 deletions(-)

-- 
Shawn.

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

* Re: [PATCH 2/3] bash completion: refactor --strategy completion
  2009-03-06 16:12     ` Jay Soffian
@ 2009-03-06 16:16       ` Shawn O. Pearce
  2009-03-06 16:30         ` Jay Soffian
  0 siblings, 1 reply; 14+ messages in thread
From: Shawn O. Pearce @ 2009-03-06 16:16 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, Markus Heidelberg, Sverre Rabbelier, Junio C Hamano

Jay Soffian <jaysoffian@gmail.com> wrote:
> On Fri, Mar 6, 2009 at 11:04 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> > Jay Soffian <jaysoffian@gmail.com> wrote:
> >> The code to complete --strategy was duplicated between _git_rebase and
> >> _git_merge, and is about to gain a third caller (_git_pull). This patch
> >> factors it into its own function.
> >>
> >> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
> >
> > Looks OK to me, but I had to squash this in to get it to work.
> > Without this the patch breaks git merge completion entirely:
> 
> Weird, I had it completely backwards but it seemed to work. Probably
> because I was sourcing the new completion into my running shell; I'll
> make sure to startup a new shell next time.
> 
> With the squash (assuming Junio doesn't mind), is it acked-by you or
> should I resend?

Sorry, yes, with the squash it is Ack'd.

-- 
Shawn.

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

* Re: [PATCH 2/3] bash completion: refactor --strategy completion
  2009-03-06 16:16       ` Shawn O. Pearce
@ 2009-03-06 16:30         ` Jay Soffian
  2009-03-06 16:32           ` Shawn O. Pearce
  0 siblings, 1 reply; 14+ messages in thread
From: Jay Soffian @ 2009-03-06 16:30 UTC (permalink / raw)
  To: git
  Cc: Jay Soffian, Markus Heidelberg, Sverre Rabbelier, Junio C Hamano,
	Shawn O . Pearce

The code to complete --strategy was duplicated between _git_rebase and
_git_merge, and is about to gain a third caller (_git_pull). This patch
factors it into its own function.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
How about like this intead? This way 3/3 doesn't need to be adjusted. The
interdiff is:

---snip---
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9e16576..056e43e 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -449,15 +449,16 @@ __git_complete_strategy ()
 	case "${COMP_WORDS[COMP_CWORD-1]}" in
 	-s|--strategy)
 		__gitcomp "$(__git_merge_strategies)"
-		return 1
+		return 0
 	esac
+	local cur="${COMP_WORDS[COMP_CWORD]}"
 	case "$cur" in
 	--strategy=*)
 		__gitcomp "$(__git_merge_strategies)" "" "${cur##--strategy=}"
-		return 1
+		return 0
 		;;
 	esac
-	return 0
+	return 1
 }
 
 __git_all_commands ()
---snap---


 contrib/completion/git-completion.bash |   38 ++++++++++++++++---------------
 1 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index e8c4be2..056e43e 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -444,6 +444,23 @@ __git_complete_remote_or_refspec ()
 	esac
 }
 
+__git_complete_strategy ()
+{
+	case "${COMP_WORDS[COMP_CWORD-1]}" in
+	-s|--strategy)
+		__gitcomp "$(__git_merge_strategies)"
+		return 0
+	esac
+	local cur="${COMP_WORDS[COMP_CWORD]}"
+	case "$cur" in
+	--strategy=*)
+		__gitcomp "$(__git_merge_strategies)" "" "${cur##--strategy=}"
+		return 0
+		;;
+	esac
+	return 1
+}
+
 __git_all_commands ()
 {
 	if [ -n "${__git_all_commandlist-}" ]; then
@@ -1095,17 +1112,10 @@ _git_log ()
 
 _git_merge ()
 {
+	__git_complete_strategy && return
+
 	local cur="${COMP_WORDS[COMP_CWORD]}"
-	case "${COMP_WORDS[COMP_CWORD-1]}" in
-	-s|--strategy)
-		__gitcomp "$(__git_merge_strategies)"
-		return
-	esac
 	case "$cur" in
-	--strategy=*)
-		__gitcomp "$(__git_merge_strategies)" "" "${cur##--strategy=}"
-		return
-		;;
 	--*)
 		__gitcomp "
 			--no-commit --no-stat --log --no-log --squash --strategy
@@ -1174,16 +1184,8 @@ _git_rebase ()
 		__gitcomp "--continue --skip --abort"
 		return
 	fi
-	case "${COMP_WORDS[COMP_CWORD-1]}" in
-	-s|--strategy)
-		__gitcomp "$(__git_merge_strategies)"
-		return
-	esac
+	__git_complete_strategy && return
 	case "$cur" in
-	--strategy=*)
-		__gitcomp "$(__git_merge_strategies)" "" "${cur##--strategy=}"
-		return
-		;;
 	--*)
 		__gitcomp "--onto --merge --strategy --interactive"
 		return
-- 
1.6.2.rc2.332.g5d21b

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

* Re: [PATCH 2/3] bash completion: refactor --strategy completion
  2009-03-06 16:30         ` Jay Soffian
@ 2009-03-06 16:32           ` Shawn O. Pearce
  0 siblings, 0 replies; 14+ messages in thread
From: Shawn O. Pearce @ 2009-03-06 16:32 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, Markus Heidelberg, Sverre Rabbelier, Junio C Hamano

Jay Soffian <jaysoffian@gmail.com> wrote:
> The code to complete --strategy was duplicated between _git_rebase and
> _git_merge, and is about to gain a third caller (_git_pull). This patch
> factors it into its own function.
> 
> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
> ---
> How about like this intead? This way 3/3 doesn't need to be adjusted. The
> interdiff is:

Yup, that works too!

Acked-by: Shawn O. Pearce <spearce@spearce.org>
 
>  contrib/completion/git-completion.bash |   38 ++++++++++++++++---------------
>  1 files changed, 20 insertions(+), 18 deletions(-)

-- 
Shawn.

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

* Re: [PATCH 1/3] bash completion: fix completion issues with fetch, pull, and push
  2009-03-06 15:58   ` Shawn O. Pearce
@ 2009-03-11 10:02     ` Nanako Shiraishi
  2009-03-11 14:57       ` Jay Soffian
  0 siblings, 1 reply; 14+ messages in thread
From: Nanako Shiraishi @ 2009-03-11 10:02 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Jay Soffian, git, Markus Heidelberg, Sverre Rabbelier,
	Junio C Hamano

Quoting Shawn O. Pearce <spearce@spearce.org>:

> Jay Soffian <jaysoffian@gmail.com> wrote:
>> Sverre Rabbelier noticed a completion issue with push:
>> 
>>  $ git push ori<tab>
>>  git push origin
>> 
>>  $ git push -f ori<tab>
>>  git push -f origin/
>> 
>> Markus Heidelberg pointed out that the issue extends to fetch and pull.
>> 
>> The reason is that the current code naively assumes that if
>> COMP_CWORD=2, it should complete a remote name, otherwise it should
>> complete a refspec. This assumption fails if there are any --options.
>> 
>> This patch fixes that issue by instead scanning COMP_CWORDS to see if
>> the remote has been completed yet (we now assume the first non-dashed
>> argument is the remote). The new logic is factored into a function,
>> shared by fetch, pull, and push.
>> 
>> The new function also properly handles '.' as the remote.
>> 
>> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
>
> Acked-by: Shawn O. Pearce <spearce@spearce.org>

While people's attention is on the completion, there is one case I wish the completion worked better.
"git log origin..mas[TAB]" and "git log origin...mas[TAB]" work as expected, but the same completion does not work for "git diff". I don't miss the two-dot format but it's often useful to say "git diff origin...master".

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

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

* Re: [PATCH 1/3] bash completion: fix completion issues with fetch,  pull, and push
  2009-03-11 10:02     ` Nanako Shiraishi
@ 2009-03-11 14:57       ` Jay Soffian
  0 siblings, 0 replies; 14+ messages in thread
From: Jay Soffian @ 2009-03-11 14:57 UTC (permalink / raw)
  To: Nanako Shiraishi
  Cc: Shawn O. Pearce, git, Markus Heidelberg, Sverre Rabbelier,
	Junio C Hamano

On Wed, Mar 11, 2009 at 6:02 AM, Nanako Shiraishi <nanako3@lavabit.com> wrote:
> While people's attention is on the completion, there is one case I wish the completion worked better.
> "git log origin..mas[TAB]" and "git log origin...mas[TAB]" work as expected, but the same completion does not work for "git diff". I don't miss the two-dot format but it's often useful to say "git diff origin...master".

I just looked and realized to fix this properly, __git_complete_file
and __git_complete_revlist need some refactoring, along with the
functions that use them.

So I'll add it to my todo list, since it's more than 5 minutes of
available time I have right now.

j.

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

end of thread, other threads:[~2009-03-11 14:58 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-06  4:39 [PATCH 0/3] improve bash completion of fetch, pull, and push Jay Soffian
2009-03-06  4:39 ` [PATCH 1/3] bash completion: fix completion issues with " Jay Soffian
2009-03-06 15:58   ` Shawn O. Pearce
2009-03-11 10:02     ` Nanako Shiraishi
2009-03-11 14:57       ` Jay Soffian
2009-03-06  4:39 ` [PATCH 2/3] bash completion: refactor --strategy completion Jay Soffian
2009-03-06 16:04   ` Shawn O. Pearce
2009-03-06 16:12     ` Jay Soffian
2009-03-06 16:16       ` Shawn O. Pearce
2009-03-06 16:30         ` Jay Soffian
2009-03-06 16:32           ` Shawn O. Pearce
2009-03-06  4:39 ` [PATCH 3/3] bash completion: teach fetch, pull, and push to complete their options Jay Soffian
2009-03-06 16:15   ` Shawn O. Pearce
2009-03-06  6:43 ` [PATCH 0/3] improve bash completion of fetch, pull, and push Sverre Rabbelier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.