{"thread":{"id":"15083","subject":"[BUG] git completion do sed on binary file","startedAt":"2008-08-19T12:27:39Z","lastAt":"2008-08-21T14:06:10Z","messageCount":16,"participants":["Nguyen Thai Ngoc Duy","Matthieu Moy","Shawn O. Pearce","Johannes Sixt","Junio C Hamano","Miklos Vajna","Mikael Magnusson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"87683","messageId":"fcaeb9bf0808190527q60869fd0uccbfd165431a752d@mail.gmail.com","threadId":"15083","inReplyTo":null,"subject":"[BUG] git completion do sed on binary file","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-08-19T12:27:39Z","receivedAt":"2008-08-19T12:27:39Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Probably missed since git-merge builtin effort:\n\n__git_merge_strategies ()\n{\n if [ -n \"$__git_merge_strategylist\" ]; then\n echo \"$__git_merge_strategylist\"\n return\n fi\n sed -n \"/^all_strategies='/{\n s/^all_strategies='//\n s/'//\n p\n q\n }\" \"$(git --exec-path)/git-merge\"\n}\n\nIt takes several seconds to finish that function.\n-- \nDuy\n"},{"id":"87688","messageId":"20080819132803.GA26201@laptop","threadId":"15083","inReplyTo":"fcaeb9bf0808190527q60869fd0uccbfd165431a752d@mail.gmail.com","subject":"[PATCH] bash-completion: fix getting strategy list","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-08-19T13:28:03Z","receivedAt":"2008-08-19T13:28:03Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Bash completion needs to know what strategies git supports. Maybe\nother similar tools have the same demand. So add \n\"git merge--show-strategies\"\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n\n On Tue, Aug 19, 2008 at 07:27:39PM +0700, Nguyen Thai Ngoc Duy wrote:\n > Probably missed since git-merge builtin effort:\n > \n > __git_merge_strategies ()\n > {\n > if [ -n \"$__git_merge_strategylist\" ]; then\n > echo \"$__git_merge_strategylist\"\n > return\n > fi\n > sed -n \"/^all_strategies='/{\n > s/^all_strategies='//\n > s/'//\n > p\n > q\n > }\" \"$(git --exec-path)/git-merge\"\n > }\n > \n > It takes several seconds to finish that function.\n\n Maybe something like this?\n\n Documentation/git-merge.txt | 4 ++++\n builtin-merge.c | 7 +++++++\n contrib/completion/git-completion.bash | 7 +------\n 3 files changed, 12 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex 17a15ac..f3fe1c9 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -12,6 +12,7 @@ SYNOPSIS\n 'git merge' [-n] [--stat] [--no-commit] [--squash] [-s ]...\n \t[-m ] ...\n 'git merge' HEAD ...\n+'git merge' --show-strategies\n \n DESCRIPTION\n -----------\n@@ -37,6 +38,9 @@ include::merge-options.txt[]\n \tleast one . Specifying more than one \n \tobviously means you are trying an Octopus.\n \n+--show-strategies::\n+\tShow all available strategies. For internal use only.\n+\n include::merge-strategies.txt[]\n \n \ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex de025ac..613c96a 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -802,6 +802,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tconst char *best_strategy = NULL, *wt_strategy = NULL;\n \tstruct commit_list **remotes = &remoteheads;\n \n+\t/* needed for git bash completion and similar tools */\n+\tif (argc == 2 && !strcmp(argv[1], \"--show-strategies\")) {\n+\t\tfor (i = 0; i < ARRAY_SIZE(all_strategy); i++)\n+\t\t\tprintf(\"%s\\n\", all_strategy[i].name);\n+\t\treturn 0;\n+\t}\n+\n \tsetup_work_tree();\n \tif (unmerged_cache())\n \t\tdie(\"You are in the middle of a conflicted merge.\");\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 158b912..1eea49a 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -271,12 +271,7 @@ __git_merge_strategies ()\n \t\techo \"$__git_merge_strategylist\"\n \t\treturn\n \tfi\n-\tsed -n \"/^all_strategies='/{\n-\t\ts/^all_strategies='//\n-\t\ts/'//\n-\t\tp\n-\t\tq\n-\t\t}\" \"$(git --exec-path)/git-merge\"\n+\t$(git --exec-path)/git-merge --show-strategies\n }\n __git_merge_strategylist=\n __git_merge_strategylist=\"$(__git_merge_strategies 2>/dev/null)\"\n-- \n1.6.0.96.g2fad1.dirty\n"},{"id":"87690","messageId":"vpqej4lcem2.fsf@bauges.imag.fr","threadId":"15083","inReplyTo":"20080819132803.GA26201@laptop","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2008-08-19T13:35:01Z","receivedAt":"2008-08-19T13:35:01Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> Bash completion needs to know what strategies git supports. Maybe\n> other similar tools have the same demand. So add \n> \"git merge--show-strategies\"\n ^^\n\nSilly detail, but a space is missing here.\n\n-- \nMatthieu\n"},{"id":"87692","messageId":"fcaeb9bf0808190646m6304b2cboa7af1d5de5e0f8d1@mail.gmail.com","threadId":"15083","inReplyTo":"vpqej4lcem2.fsf@bauges.imag.fr","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-08-19T13:46:44Z","receivedAt":"2008-08-19T13:46:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 8/19/08, Matthieu Moy wrote:\n> Nguyen Thai Ngoc Duy writes:\n>\n> > Bash completion needs to know what strategies git supports. Maybe\n> > other similar tools have the same demand. So add\n> > \"git merge--show-strategies\"\n>\n> ^^\n>\n> Silly detail, but a space is missing here.\n\nYes, my poor eyes. Junio if you accept the patch, please add one more\nspace. Thanks :)\n\n-- \nDuy\n"},{"id":"87695","messageId":"20080819141832.GA20947@spearce.org","threadId":"15083","inReplyTo":"20080819132803.GA26201@laptop","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-08-19T14:18:32Z","receivedAt":"2008-08-19T14:18:32Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Nguyen Thai Ngoc Duy wrote:\n> > __git_merge_strategies ()\n> > {\n> > if [ -n \"$__git_merge_strategylist\" ]; then\n> > echo \"$__git_merge_strategylist\"\n> > return\n> > fi\n> > sed -n \"/^all_strategies='/{\n> > s/^all_strategies='//\n> > s/'//\n> > p\n> > q\n> > }\" \"$(git --exec-path)/git-merge\"\n> > }\n> > \n> > It takes several seconds to finish that function.\n\nYouch.\n \n> Maybe something like this?\n\nYea, this looks reasonable. ACK from me, though the bigger change\nis to builtin-merge and not the completion. Miklos? Dscho?\n\n> diff --git a/builtin-merge.c b/builtin-merge.c\n> index de025ac..613c96a 100644\n> --- a/builtin-merge.c\n> +++ b/builtin-merge.c\n> @@ -802,6 +802,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n> \tconst char *best_strategy = NULL, *wt_strategy = NULL;\n> \tstruct commit_list **remotes = &remoteheads;\n> \n> +\t/* needed for git bash completion and similar tools */\n> +\tif (argc == 2 && !strcmp(argv[1], \"--show-strategies\")) {\n> +\t\tfor (i = 0; i < ARRAY_SIZE(all_strategy); i++)\n> +\t\t\tprintf(\"%s\\n\", all_strategy[i].name);\n> +\t\treturn 0;\n> +\t}\n> +\n> \tsetup_work_tree();\n> \tif (unmerged_cache())\n> \t\tdie(\"You are in the middle of a conflicted merge.\");\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 158b912..1eea49a 100755\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -271,12 +271,7 @@ __git_merge_strategies ()\n> \t\techo \"$__git_merge_strategylist\"\n> \t\treturn\n> \tfi\n> -\tsed -n \"/^all_strategies='/{\n> -\t\ts/^all_strategies='//\n> -\t\ts/'//\n> -\t\tp\n> -\t\tq\n> -\t\t}\" \"$(git --exec-path)/git-merge\"\n> +\t$(git --exec-path)/git-merge --show-strategies\n> }\n> __git_merge_strategylist=\n> __git_merge_strategylist=\"$(__git_merge_strategies 2>/dev/null)\"\n\n-- \nShawn.\n"},{"id":"87701","messageId":"48AADDBB.1080203@viscovery.net","threadId":"15083","inReplyTo":"20080819132803.GA26201@laptop","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-08-19T14:50:35Z","receivedAt":"2008-08-19T14:50:35Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Nguyen Thai Ngoc Duy schrieb:\n> +--show-strategies::\n> +\tShow all available strategies. For internal use only.\n> +\n\nIMO, you don't need to declare this option as internal; offering it for\nthe public is fine...\n\n> +\t/* needed for git bash completion and similar tools */\n\n... which would make this comment slightly odd.\n\n> +\tif (argc == 2 && !strcmp(argv[1], \"--show-strategies\")) {\n> +\t\tfor (i = 0; i < ARRAY_SIZE(all_strategy); i++)\n> +\t\t\tprintf(\"%s\\n\", all_strategy[i].name);\n> +\t\treturn 0;\n\nImproved error checking, but quick and dirty:\n\n+\tif (!strcmp(argv[1], \"--show-strategies\")) {\n+\t\tfor (i = 0; i < ARRAY_SIZE(all_strategy); i++)\n+\t\t\tprintf(\"%s\\n\", all_strategy[i].name);\n+\t\treturn argc == 2 ? 0 :\n+\t\t\t error(\"--show-strategies does not take \"\n+\t\t\t\t\"any arguments\");\n\n> +\t$(git --exec-path)/git-merge --show-strategies\n\n+\tgit merge --show-strategies\n\n-- Hannes\n"},{"id":"87704","messageId":"48AADF3E.40304@viscovery.net","threadId":"15083","inReplyTo":"48AADDBB.1080203@viscovery.net","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2008-08-19T14:57:02Z","receivedAt":"2008-08-19T14:57:02Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Sixt schrieb:\n> Nguyen Thai Ngoc Duy schrieb:\n>> +\tif (argc == 2 && !strcmp(argv[1], \"--show-strategies\")) {\n>> +\t\tfor (i = 0; i < ARRAY_SIZE(all_strategy); i++)\n>> +\t\t\tprintf(\"%s\\n\", all_strategy[i].name);\n>> +\t\treturn 0;\n> \n> Improved error checking, but quick and dirty:\n> \n> +\tif (!strcmp(argv[1], \"--show-strategies\")) {\n\nOops, not really improved. This still needs to check for argc >= 2.\n\n> +\t\tfor (i = 0; i < ARRAY_SIZE(all_strategy); i++)\n> +\t\t\tprintf(\"%s\\n\", all_strategy[i].name);\n> +\t\treturn argc == 2 ? 0 :\n> +\t\t\t error(\"--show-strategies does not take \"\n> +\t\t\t\t\"any arguments\");\n\n-- Hannes\n"},{"id":"87827","messageId":"fcaeb9bf0808200958u65ad8fa3oa118b88e16c9c50c@mail.gmail.com","threadId":"15083","inReplyTo":"48AADDBB.1080203@viscovery.net","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-08-20T16:58:14Z","receivedAt":"2008-08-20T16:58:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 8/19/08, Johannes Sixt wrote:\n> Nguyen Thai Ngoc Duy schrieb:\n>\n> > +--show-strategies::\n> > + Show all available strategies. For internal use only.\n> > +\n>\n>\n> IMO, you don't need to declare this option as internal; offering it for\n> the public is fine...\n\nOn second thought, I don't think the patch's worth it. The code in\ngit-completion.bash is a hack and I replace it with another the hack.\nIt won't work for custom merges and git-completion.bash will need to\nbe synced manually anyway, so maybe this patch will do better:\n\ndiff --git a/contrib/completion/git-completion.bash\nb/contrib/completion/git-completion.bash\nindex 158b912..2fed6ac 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -267,19 +267,8 @@ __git_remotes ()\n\n __git_merge_strategies ()\n {\n-\tif [ -n \"$__git_merge_strategylist\" ]; then\n-\t\techo \"$__git_merge_strategylist\"\n-\t\treturn\n-\tfi\n-\tsed -n \"/^all_strategies='/{\n-\t\ts/^all_strategies='//\n-\t\ts/'//\n-\t\tp\n-\t\tq\n-\t\t}\" \"$(git --exec-path)/git-merge\"\n-}\n-__git_merge_strategylist=\n-__git_merge_strategylist=\"$(__git_merge_strategies 2>/dev/null)\"\n+\techo recursive octopus resolve ours subtree\n+}\n\n __git_complete_file ()\n {\n\n-- \nDuy\n"},{"id":"87839","messageId":"7v3akzsg0c.fsf@gitster.siamese.dyndns.org","threadId":"15083","inReplyTo":"fcaeb9bf0808200958u65ad8fa3oa118b88e16c9c50c@mail.gmail.com","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-20T18:22:43Z","receivedAt":"2008-08-20T18:22:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Nguyen Thai Ngoc Duy\" writes:\n\n> On second thought, I don't think the patch's worth it. The code in\n> git-completion.bash is a hack and I replace it with another the hack.\n> It won't work for custom merges and git-completion.bash will need to\n> be synced manually anyway, so maybe this patch will do better:\n\nThis would be the right thing to do for the 'maint' track. In the longer\nterm, especially if we are to actually make the \"custom\" thing official, I\nthink teaching \"git-merge\" to report what are available is the only\nsensible approach, though.\n"},{"id":"87879","messageId":"7vfxozpeyh.fsf@gitster.siamese.dyndns.org","threadId":"15083","inReplyTo":"fcaeb9bf0808200958u65ad8fa3oa118b88e16c9c50c@mail.gmail.com","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-20T21:13:42Z","receivedAt":"2008-08-20T21:13:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Nguyen Thai Ngoc Duy\" writes:\n\n> On 8/19/08, Johannes Sixt wrote:\n>> Nguyen Thai Ngoc Duy schrieb:\n>>\n>> > +--show-strategies::\n>> > + Show all available strategies. For internal use only.\n>> > +\n>>\n>>\n>> IMO, you don't need to declare this option as internal; offering it for\n>> the public is fine...\n>\n> On second thought, I don't think the patch's worth it.\n\nHow about doing this instead?\n\n-- >8 --\ncompletion: find out supported merge strategies correctly\n\n\"git-merge\" is a binary executable these days, and looking for assignment\nto $all_strategies variable does not work well.\n\nWhen asked for an unknown strategy, pre-1.6.0 and post-1.6.0 \"git merge\"\ncommands respectively say:\n\n $ $HOME/git-snap-v1.5.6.5/bin/git merge -s help\n available strategies are: recur recursive octopus resolve stupid ours subtree \n $ $HOME/git-snap-v1.6.0/bin/git merge -s help\n Could not find merge strategy 'help'.\n Available strategies are: recursive octopus resolve ours subtree.\n\nboth on its standard error stream. We can use this to learn what\nstrategies are supported.\n\nThe sed script is written in such a way that it catches both old and new\nmessage styles (\"Available\" vs \"available\", and the full stop at the end).\nIt also allows future versions of \"git merge\" to line-wrap the list of\nstrategies, and add extra comments, like this:\n\n $ $HOME/git-snap-v1.6.1/bin/git merge -s help\n Could not find merge strategy 'help'.\n Available strategies are: blame recursive octopus resolve ours\n subtree.\n Also you have custom strategies: theirs\n\n Make sure you spell strategy names correctly.\n\n---\ndiff --git c/contrib/completion/git-completion.bash w/contrib/completion/git-completion.bash\nindex 158b912..a310040 100755\n--- c/contrib/completion/git-completion.bash\n+++ w/contrib/completion/git-completion.bash\n@@ -271,15 +271,17 @@ __git_merge_strategies ()\n \t\techo \"$__git_merge_strategylist\"\n \t\treturn\n \tfi\n-\tsed -n \"/^all_strategies='/{\n-\t\ts/^all_strategies='//\n-\t\ts/'//\n+\tgit merge -s help 2>&1 |\n+\tsed -n -e '/[Aa]vailable strategies are: /,/^$/{\n+\t\ts/\\.$//\n+\t\ts/.*://\n+\t\ts/^[ \t]*//\n+\t\ts/[ \t]*$//\n \t\tp\n-\t\tq\n-\t\t}\" \"$(git --exec-path)/git-merge\"\n+\t}'\n }\n __git_merge_strategylist=\n-__git_merge_strategylist=\"$(__git_merge_strategies 2>/dev/null)\"\n+__git_merge_strategylist=$(__git_merge_strategies 2>/dev/null)\n \n __git_complete_file ()\n {\n"},{"id":"87900","messageId":"20080820223938.GN23800@genesis.frugalware.org","threadId":"15083","inReplyTo":"7vfxozpeyh.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-20T22:39:38Z","receivedAt":"2008-08-20T22:39:38Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Wed, Aug 20, 2008 at 02:13:42PM -0700, Junio C Hamano wrote:\n> completion: find out supported merge strategies correctly\n> \n> \"git-merge\" is a binary executable these days, and looking for assignment\n> to $all_strategies variable does not work well.\n> \n> When asked for an unknown strategy, pre-1.6.0 and post-1.6.0 \"git merge\"\n> commands respectively say:\n> \n> $ $HOME/git-snap-v1.5.6.5/bin/git merge -s help\n> available strategies are: recur recursive octopus resolve stupid ours subtree \n> $ $HOME/git-snap-v1.6.0/bin/git merge -s help\n> Could not find merge strategy 'help'.\n> Available strategies are: recursive octopus resolve ours subtree.\n> \n> both on its standard error stream. We can use this to learn what\n> strategies are supported.\n> \n> The sed script is written in such a way that it catches both old and new\n> message styles (\"Available\" vs \"available\", and the full stop at the end).\n> It also allows future versions of \"git merge\" to line-wrap the list of\n> strategies, and add extra comments, like this:\n> \n> $ $HOME/git-snap-v1.6.1/bin/git merge -s help\n> Could not find merge strategy 'help'.\n> Available strategies are: blame recursive octopus resolve ours\n> subtree.\n> Also you have custom strategies: theirs\n> \n> Make sure you spell strategy names correctly.\n\nI like this, not adding something to builtin-merge when it's already\nthere. :)\n"},{"id":"87912","messageId":"fcaeb9bf0808201743v6a5144c7u79c0a0ecd3416b69@mail.gmail.com","threadId":"15083","inReplyTo":"20080820223938.GN23800@genesis.frugalware.org","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2008-08-21T00:43:23Z","receivedAt":"2008-08-21T00:43:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 8/21/08, Miklos Vajna wrote:\n> On Wed, Aug 20, 2008 at 02:13:42PM -0700, Junio C Hamano wrote:\n> > completion: find out supported merge strategies correctly\n> >\n> > \"git-merge\" is a binary executable these days, and looking for assignment\n> > to $all_strategies variable does not work well.\n> >\n> > When asked for an unknown strategy, pre-1.6.0 and post-1.6.0 \"git merge\"\n> > commands respectively say:\n> >\n> > $ $HOME/git-snap-v1.5.6.5/bin/git merge -s help\n> > available strategies are: recur recursive octopus resolve stupid ours subtree\n> > $ $HOME/git-snap-v1.6.0/bin/git merge -s help\n> > Could not find merge strategy 'help'.\n> > Available strategies are: recursive octopus resolve ours subtree.\n> >\n> > both on its standard error stream. We can use this to learn what\n> > strategies are supported.\n> >\n> > The sed script is written in such a way that it catches both old and new\n> > message styles (\"Available\" vs \"available\", and the full stop at the end).\n> > It also allows future versions of \"git merge\" to line-wrap the list of\n> > strategies, and add extra comments, like this:\n> >\n> > $ $HOME/git-snap-v1.6.1/bin/git merge -s help\n> > Could not find merge strategy 'help'.\n> > Available strategies are: blame recursive octopus resolve ours\n> > subtree.\n> > Also you have custom strategies: theirs\n> >\n> > Make sure you spell strategy names correctly.\n>\n>\n> I like this, not adding something to builtin-merge when it's already\n> there. :)\n\nMe too. Did not realize it's already there :P\n\n-- \nDuy\n"},{"id":"87920","messageId":"237967ef0808201914k2cf8ad09s7b7d56dd8ea35d62@mail.gmail.com","threadId":"15083","inReplyTo":"7vfxozpeyh.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Mikael Magnusson","fromEmail":"mikachu@gmail.com","sentAt":"2008-08-21T02:14:09Z","receivedAt":"2008-08-21T02:14:09Z","isPatch":true,"sender":{"key":"mikachu@gmail.com","avatar":null},"body":"2008/8/20 Junio C Hamano :\n> \"Nguyen Thai Ngoc Duy\" writes:\n>\n>> On 8/19/08, Johannes Sixt wrote:\n>>> Nguyen Thai Ngoc Duy schrieb:\n>>>\n>>> > +--show-strategies::\n>>> > + Show all available strategies. For internal use only.\n>>> > +\n>>>\n>>>\n>>> IMO, you don't need to declare this option as internal; offering it for\n>>> the public is fine...\n>>\n>> On second thought, I don't think the patch's worth it.\n>\n> How about doing this instead?\n>\n> -- >8 --\n> completion: find out supported merge strategies correctly\n>\n> \"git-merge\" is a binary executable these days, and looking for assignment\n> to $all_strategies variable does not work well.\n>\n> When asked for an unknown strategy, pre-1.6.0 and post-1.6.0 \"git merge\"\n> commands respectively say:\n>\n> $ $HOME/git-snap-v1.5.6.5/bin/git merge -s help\n> available strategies are: recur recursive octopus resolve stupid ours subtree\n> $ $HOME/git-snap-v1.6.0/bin/git merge -s help\n> Could not find merge strategy 'help'.\n> Available strategies are: recursive octopus resolve ours subtree.\n>\n> both on its standard error stream. We can use this to learn what\n> strategies are supported.\n>\n> The sed script is written in such a way that it catches both old and new\n> message styles (\"Available\" vs \"available\", and the full stop at the end).\n> It also allows future versions of \"git merge\" to line-wrap the list of\n> strategies, and add extra comments, like this:\n>\n> $ $HOME/git-snap-v1.6.1/bin/git merge -s help\n> Could not find merge strategy 'help'.\n> Available strategies are: blame recursive octopus resolve ours\n> subtree.\n> Also you have custom strategies: theirs\n>\n> Make sure you spell strategy names correctly.\n>\n> ---\n> diff --git c/contrib/completion/git-completion.bash w/contrib/completion/git-completion.bash\n> index 158b912..a310040 100755\n> --- c/contrib/completion/git-completion.bash\n> +++ w/contrib/completion/git-completion.bash\n> @@ -271,15 +271,17 @@ __git_merge_strategies ()\n> echo \"$__git_merge_strategylist\"\n> return\n> fi\n> - sed -n \"/^all_strategies='/{\n> - s/^all_strategies='//\n> - s/'//\n> + git merge -s help 2>&1 |\n> + sed -n -e '/[Aa]vailable strategies are: /,/^$/{\n> + s/\\.$//\n> + s/.*://\n> + s/^[ ]*//\n> + s/[ ]*$//\n> p\n> - q\n> - }\" \"$(git --exec-path)/git-merge\"\n> + }'\n> }\n> __git_merge_strategylist=\n> -__git_merge_strategylist=\"$(__git_merge_strategies 2>/dev/null)\"\n> +__git_merge_strategylist=$(__git_merge_strategies 2>/dev/null)\n>\n> __git_complete_file ()\n> {\n\nI don't know if i somehow have some weird patch merged in, i tried to check\nand didn't find anything, at least. But when i run git merge -s help i get:\n\nCould not find merge strategy 'help'.\n\navailable strategies in '/usr/libexec/git-core'\n-----------------------------------------------\n file octopus ours recursive resolve subtree\n\nwhich i suspect does not work with your regex.\n\n-- \nMikael Magnusson\n"},{"id":"87942","messageId":"7vbpznkmbf.fsf@gitster.siamese.dyndns.org","threadId":"15083","inReplyTo":"237967ef0808201914k2cf8ad09s7b7d56dd8ea35d62@mail.gmail.com","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-21T04:45:56Z","receivedAt":"2008-08-21T04:45:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Mikael Magnusson\" writes:\n\n> I don't know if i somehow have some weird patch merged in, i tried to check\n> and didn't find anything, at least. But when i run git merge -s help i get:\n\nYes, there are some weird patch in 'next'. Miklos, could you fix the\noutput format of that thing?\n"},{"id":"87943","messageId":"7v7iabklas.fsf@gitster.siamese.dyndns.org","threadId":"15083","inReplyTo":"7vbpznkmbf.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-21T05:07:55Z","receivedAt":"2008-08-21T05:07:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"Mikael Magnusson\" writes:\n>\n>> I don't know if i somehow have some weird patch merged in, i tried to check\n>> and didn't find anything, at least. But when i run git merge -s help i get:\n>\n> Yes, there are some weird patch in 'next'. Miklos, could you fix the\n> output format of that thing?\n\nI do not think the code in 'next' that reuses the \"help\" thing to show\nonly the list to stdout while still giving the error message to stderr\nmakes any sense.\n\nLet's do this.\n\n---\n\n builtin-merge.c | 14 +++++++++++---\n 1 files changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git c/builtin-merge.c w/builtin-merge.c\nindex 1f9389b..3e8db0d 100644\n--- c/builtin-merge.c\n+++ w/builtin-merge.c\n@@ -110,9 +110,17 @@ static struct strategy *get_strategy(const char *name)\n \t\t}\n \t}\n \tif (!is_in_cmdlist(&main_cmds, name) && !is_in_cmdlist(&other_cmds, name)) {\n-\n-\t\tfprintf(stderr, \"Could not find merge strategy '%s'.\\n\\n\", name);\n-\t\tlist_commands(\"strategies\", longest, &main_cmds, &other_cmds);\n+\t\tfprintf(stderr, \"Could not find merge strategy '%s'.\\n\", name);\n+\t\tfprintf(stderr, \"Available strategies are:\");\n+\t\tfor (i = 0; i < main_cmds.cnt; i++)\n+\t\t\tfprintf(stderr, \" %s\", main_cmds.names[i]->name);\n+\t\tfprintf(stderr, \".\\n\");\n+\t\tif (other_cmds.cnt) {\n+\t\t\tfprintf(stderr, \"Available custom strategies are:\");\n+\t\t\tfor (i = 0; i < other_cmds.cnt; i++)\n+\t\t\t\tfprintf(stderr, \" %s\", other_cmds.names[i]->name);\n+\t\t\tfprintf(stderr, \".\\n\");\n+\t\t}\n \t\texit(1);\n \t}\n \n"},{"id":"87982","messageId":"20080821140610.GS23800@genesis.frugalware.org","threadId":"15083","inReplyTo":"7v7iabklas.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] bash-completion: fix getting strategy list","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-21T14:06:10Z","receivedAt":"2008-08-21T14:06:10Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Wed, Aug 20, 2008 at 10:07:55PM -0700, Junio C Hamano wrote:\n> > Yes, there are some weird patch in 'next'. Miklos, could you fix the\n> > output format of that thing?\n\nYou were faster. ;-)\n\n> I do not think the code in 'next' that reuses the \"help\" thing to show\n> only the list to stdout while still giving the error message to stderr\n> makes any sense.\n> \n> Let's do this.\n> \n> ---\n> \n> builtin-merge.c | 14 +++++++++++---\n> 1 files changed, 11 insertions(+), 3 deletions(-)\n> \n> diff --git c/builtin-merge.c w/builtin-merge.c\n> index 1f9389b..3e8db0d 100644\n> --- c/builtin-merge.c\n> +++ w/builtin-merge.c\n> @@ -110,9 +110,17 @@ static struct strategy *get_strategy(const char *name)\n> \t\t}\n> \t}\n> \tif (!is_in_cmdlist(&main_cmds, name) && !is_in_cmdlist(&other_cmds, name)) {\n> -\n> -\t\tfprintf(stderr, \"Could not find merge strategy '%s'.\\n\\n\", name);\n> -\t\tlist_commands(\"strategies\", longest, &main_cmds, &other_cmds);\n> +\t\tfprintf(stderr, \"Could not find merge strategy '%s'.\\n\", name);\n> +\t\tfprintf(stderr, \"Available strategies are:\");\n> +\t\tfor (i = 0; i < main_cmds.cnt; i++)\n> +\t\t\tfprintf(stderr, \" %s\", main_cmds.names[i]->name);\n> +\t\tfprintf(stderr, \".\\n\");\n> +\t\tif (other_cmds.cnt) {\n> +\t\t\tfprintf(stderr, \"Available custom strategies are:\");\n> +\t\t\tfor (i = 0; i < other_cmds.cnt; i++)\n> +\t\t\t\tfprintf(stderr, \" %s\", other_cmds.names[i]->name);\n> +\t\t\tfprintf(stderr, \".\\n\");\n> +\t\t}\n> \t\texit(1);\n> \t}\n\nGiven that we don't have 100+ merge strategies like we have 100+ git\ncommands, I think printing them in one line should not be problematic,\nand this definitely improves script usability, so I like it, thanks.\n"}]}