[BUG] git completion do sed on binary file

Probably missed since git-merge builtin effort:

__git_merge_strategies ()
{
        if [ -n "$__git_merge_strategylist" ]; then
                echo "$__git_merge_strategylist"
                return
        fi
        sed -n "/^all_strategies='/{
                s/^all_strategies='//
                s/'//
                p
                q
                }" "$(git --exec-path)/git-merge"
}

It takes several seconds to finish that function.
-- 
Duy

Bash completion needs to know what strategies git supports. Maybe
other similar tools have the same demand. So add 
"git merge--show-strategies"

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---

  On Tue, Aug 19, 2008 at 07:27:39PM +0700, Nguyen Thai Ngoc Duy wrote:
  > Probably missed since git-merge builtin effort:
  > 
  > __git_merge_strategies ()
  > {
  >         if [ -n "$__git_merge_strategylist" ]; then
  >                 echo "$__git_merge_strategylist"
  >                 return
  >         fi
  >         sed -n "/^all_strategies='/{
  >                 s/^all_strategies='//
  >                 s/'//
  >                 p
  >                 q
  >                 }" "$(git --exec-path)/git-merge"
  > }
  > 
  > It takes several seconds to finish that function.

  Maybe something like this?

 Documentation/git-merge.txt            |    4 ++++
 builtin-merge.c                        |    7 +++++++
 contrib/completion/git-completion.bash |    7 +------
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 17a15ac..f3fe1c9 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt

@@ -12,6 +12,7 @@ SYNOPSIS
 'git merge' [-n] [--stat] [--no-commit] [--squash] [-s <strategy>]...
 	[-m <msg>] <remote> <remote>...
 'git merge' <msg> HEAD <remote>...
+'git merge' --show-strategies
 
 DESCRIPTION
 -----------

@@ -37,6 +38,9 @@ include::merge-options.txt[]
 	least one <remote>.  Specifying more than one <remote>
 	obviously means you are trying an Octopus.
 
+--show-strategies::
+	Show all available strategies. For internal use only.
+
 include::merge-strategies.txt[]
 
 

diff --git a/builtin-merge.c b/builtin-merge.c
index de025ac..613c96a 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c

@@ -802,6 +802,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	const char *best_strategy = NULL, *wt_strategy = NULL;
 	struct commit_list **remotes = &remoteheads;
 
+	/* needed for git bash completion and similar tools */
+	if (argc == 2 && !strcmp(argv[1], "--show-strategies")) {
+		for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
+			printf("%s\n", all_strategy[i].name);
+		return 0;
+	}
+
 	setup_work_tree();
 	if (unmerged_cache())
 		die("You are in the middle of a conflicted merge.");

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 158b912..1eea49a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -271,12 +271,7 @@ __git_merge_strategies ()
 		echo "$__git_merge_strategylist"
 		return
 	fi
-	sed -n "/^all_strategies='/{
-		s/^all_strategies='//
-		s/'//
-		p
-		q
-		}" "$(git --exec-path)/git-merge"
+	$(git --exec-path)/git-merge --show-strategies
 }
 __git_merge_strategylist=
 __git_merge_strategylist="$(__git_merge_strategies 2>/dev/null)"

-- 
1.6.0.96.g2fad1.dirty

Nguyen Thai Ngoc Duy [off-list ref] wrote:

  > __git_merge_strategies ()
  > {
  >         if [ -n "$__git_merge_strategylist" ]; then
  >                 echo "$__git_merge_strategylist"
  >                 return
  >         fi
  >         sed -n "/^all_strategies='/{
  >                 s/^all_strategies='//
  >                 s/'//
  >                 p
  >                 q
  >                 }" "$(git --exec-path)/git-merge"
  > }
  > 
  > It takes several seconds to finish that function.

Youch.
 

  Maybe something like this?

Yea, this looks reasonable.  ACK from me, though the bigger change
is to builtin-merge and not the completion. Miklos?  Dscho?

-- 
Shawn.

Nguyen Thai Ngoc Duy schrieb:

+--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...

+	/* needed for git bash completion and similar tools */

... which would make this comment slightly odd.

+	if (argc == 2 && !strcmp(argv[1], "--show-strategies")) {
+		for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
+			printf("%s\n", all_strategy[i].name);
+		return 0;

Improved error checking, but quick and dirty:

+	if (!strcmp(argv[1], "--show-strategies")) {
+		for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
+			printf("%s\n", all_strategy[i].name);
+		return argc == 2 ? 0 :
+			 error("--show-strategies does not take "
+				"any arguments");

+	$(git --exec-path)/git-merge --show-strategies

+	git merge --show-strategies

-- Hannes

Johannes Sixt schrieb:

Nguyen Thai Ngoc Duy schrieb:

quoted

+	if (argc == 2 && !strcmp(argv[1], "--show-strategies")) {
+		for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
+			printf("%s\n", all_strategy[i].name);
+		return 0;

Improved error checking, but quick and dirty:

+	if (!strcmp(argv[1], "--show-strategies")) {

Oops, not really improved. This still needs to check for argc >= 2.

+		for (i = 0; i < ARRAY_SIZE(all_strategy); i++)
+			printf("%s\n", all_strategy[i].name);
+		return argc == 2 ? 0 :
+			 error("--show-strategies does not take "
+				"any arguments");

-- Hannes

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

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. The code in
git-completion.bash is a hack and I replace it with another the hack.
It won't work for custom merges and git-completion.bash will need to
be synced manually anyway, so maybe this patch will do better:

diff --git a/contrib/completion/git-completion.bash

b/contrib/completion/git-completion.bash
index 158b912..2fed6ac 100755

--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -267,19 +267,8 @@ __git_remotes ()

 __git_merge_strategies ()
 {
-	if [ -n "$__git_merge_strategylist" ]; then
-		echo "$__git_merge_strategylist"
-		return
-	fi
-	sed -n "/^all_strategies='/{
-		s/^all_strategies='//
-		s/'//
-		p
-		q
-		}" "$(git --exec-path)/git-merge"
-}
-__git_merge_strategylist=
-__git_merge_strategylist="$(__git_merge_strategies 2>/dev/null)"
+	echo recursive octopus resolve ours subtree
+}

 __git_complete_file ()
 {

-- 
Duy