git.vger.kernel.org archive mirror
 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 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 NNTP newsgroup(s).