Re: [PATCH] bash-completion: fix getting strategy list

"Nguyen Thai Ngoc Duy" [off-list ref] writes:

On 8/19/08, Johannes Sixt [off-list ref] wrote:

quoted

Nguyen Thai Ngoc Duy schrieb:

quoted

+--show-strategies::

 > +     Show all available strategies. For internal use only.
 > +


IMO, you don't need to declare this option as internal; offering it for
 the public is fine...

On second thought, I don't think the patch's worth it.

How about doing this instead?

-- >8 --
completion: find out supported merge strategies correctly

"git-merge" is a binary executable these days, and looking for assignment
to $all_strategies variable does not work well.

When asked for an unknown strategy, pre-1.6.0 and post-1.6.0 "git merge"
commands respectively say:

    $ $HOME/git-snap-v1.5.6.5/bin/git merge -s help
    available strategies are: recur recursive octopus resolve stupid ours subtree 
    $ $HOME/git-snap-v1.6.0/bin/git merge -s help
    Could not find merge strategy 'help'.
    Available strategies are: recursive octopus resolve ours subtree.

both on its standard error stream.  We can use this to learn what
strategies are supported.

The sed script is written in such a way that it catches both old and new
message styles ("Available" vs "available", and the full stop at the end).
It also allows future versions of "git merge" to line-wrap the list of
strategies, and add extra comments, like this:

    $ $HOME/git-snap-v1.6.1/bin/git merge -s help
    Could not find merge strategy 'help'.
    Available strategies are: blame recursive octopus resolve ours
    subtree.
    Also you have custom strategies: theirs

    Make sure you spell strategy names correctly.

---

diff --git c/contrib/completion/git-completion.bash w/contrib/completion/git-completion.bash
index 158b912..a310040 100755
--- c/contrib/completion/git-completion.bash
+++ w/contrib/completion/git-completion.bash

@@ -271,15 +271,17 @@ __git_merge_strategies ()
 		echo "$__git_merge_strategylist"
 		return
 	fi
-	sed -n "/^all_strategies='/{
-		s/^all_strategies='//
-		s/'//
+	git merge -s help 2>&1 |
+	sed -n -e '/[Aa]vailable strategies are: /,/^$/{
+		s/\.$//
+		s/.*://
+		s/^[ 	]*//
+		s/[ 	]*$//
 		p
-		q
-		}" "$(git --exec-path)/git-merge"
+	}'
 }
 __git_merge_strategylist=
-__git_merge_strategylist="$(__git_merge_strategies 2>/dev/null)"
+__git_merge_strategylist=$(__git_merge_strategies 2>/dev/null)
 
 __git_complete_file ()
 {

On Wed, Aug 20, 2008 at 02:13:42PM -0700, Junio C Hamano [off-list ref] wrote:

completion: find out supported merge strategies correctly

"git-merge" is a binary executable these days, and looking for assignment
to $all_strategies variable does not work well.

When asked for an unknown strategy, pre-1.6.0 and post-1.6.0 "git merge"
commands respectively say:

    $ $HOME/git-snap-v1.5.6.5/bin/git merge -s help
    available strategies are: recur recursive octopus resolve stupid ours subtree 
    $ $HOME/git-snap-v1.6.0/bin/git merge -s help
    Could not find merge strategy 'help'.
    Available strategies are: recursive octopus resolve ours subtree.

both on its standard error stream.  We can use this to learn what
strategies are supported.

The sed script is written in such a way that it catches both old and new
message styles ("Available" vs "available", and the full stop at the end).
It also allows future versions of "git merge" to line-wrap the list of
strategies, and add extra comments, like this:

    $ $HOME/git-snap-v1.6.1/bin/git merge -s help
    Could not find merge strategy 'help'.
    Available strategies are: blame recursive octopus resolve ours
    subtree.
    Also you have custom strategies: theirs

    Make sure you spell strategy names correctly.

I like this, not adding something to builtin-merge when it's already
there. :)

On 8/21/08, Miklos Vajna [off-list ref] wrote:

On Wed, Aug 20, 2008 at 02:13:42PM -0700, Junio C Hamano [off-list ref] wrote:
 > completion: find out supported merge strategies correctly
 >
 > "git-merge" is a binary executable these days, and looking for assignment
 > to $all_strategies variable does not work well.
 >
 > When asked for an unknown strategy, pre-1.6.0 and post-1.6.0 "git merge"
 > commands respectively say:
 >
 >     $ $HOME/git-snap-v1.5.6.5/bin/git merge -s help
 >     available strategies are: recur recursive octopus resolve stupid ours subtree
 >     $ $HOME/git-snap-v1.6.0/bin/git merge -s help
 >     Could not find merge strategy 'help'.
 >     Available strategies are: recursive octopus resolve ours subtree.
 >
 > both on its standard error stream.  We can use this to learn what
 > strategies are supported.
 >
 > The sed script is written in such a way that it catches both old and new
 > message styles ("Available" vs "available", and the full stop at the end).
 > It also allows future versions of "git merge" to line-wrap the list of
 > strategies, and add extra comments, like this:
 >
 >     $ $HOME/git-snap-v1.6.1/bin/git merge -s help
 >     Could not find merge strategy 'help'.
 >     Available strategies are: blame recursive octopus resolve ours
 >     subtree.
 >     Also you have custom strategies: theirs
 >
 >     Make sure you spell strategy names correctly.


I like this, not adding something to builtin-merge when it's already
 there. :)

Me too. Did not realize it's already there :P

-- 
Duy

2008/8/20 Junio C Hamano [off-list ref]:

I don't know if i somehow have some weird patch merged in, i tried to check
and didn't find anything, at least. But when i run git merge -s help i get:

Could not find merge strategy 'help'.

available strategies in '/usr/libexec/git-core'
-----------------------------------------------
  file      octopus   ours      recursive resolve   subtree

which i suspect does not work with your regex.

-- 
Mikael Magnusson