{"thread":{"id":"47688","subject":"[PATCH] merge: support --strategy '?' for git-completion.bash","startedAt":"2018-01-25T09:40:47Z","lastAt":"2018-01-26T17:47:48Z","messageCount":4,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"337357","messageId":"20180125094031.6584-1-pclouds@gmail.com","threadId":"47688","inReplyTo":null,"subject":"[PATCH] merge: support --strategy '?' for git-completion.bash","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-01-25T09:40:31Z","receivedAt":"2018-01-25T09:40:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Bash completion support gets the list of available strategies with a\ngrep and sed trick which does not work on non-C locale since the anchor\nstring is translated and it does not cover custom strategies either.\n\nLet's do it a better way and make git-merge provide all available\nstrategies in machine-readable form.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Another, perhaps better, option is add \"git merge --list-strategies\".\n It requires some code movement, so I'll try with a simpler approach\n first.\n\n Documentation/merge-options.txt        | 3 +++\n builtin/merge.c                        | 7 +++++++\n contrib/completion/git-completion.bash | 9 +--------\n 3 files changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt\nindex 3888c3ff85..cd4342844f 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -97,6 +97,9 @@ option can be used to override --squash.\n \tIf there is no `-s` option, a built-in list of strategies\n \tis used instead ('git merge-recursive' when merging a single\n \thead, 'git merge-octopus' otherwise).\n++\n+The special strategy '?' lists all available strategies and exits\n+immediately. No merge operation is done.\n \n -X <option>::\n --strategy-option=<option>::\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 30264cfd7c..a09d04302c 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -140,6 +140,13 @@ static struct strategy *get_strategy(const char *name)\n \t\t}\n \t\texclude_cmds(&main_cmds, &not_strategies);\n \t}\n+\tif (!strcmp(name, \"?\")) {\n+\t\tfor (i = 0; i < main_cmds.cnt; i++)\n+\t\t\tputs(main_cmds.names[i]->name);\n+\t\tfor (i = 0; i < other_cmds.cnt; i++)\n+\t\t\tputs(other_cmds.names[i]->name);\n+\t\texit(0);\n+\t}\n \tif (!is_in_cmdlist(&main_cmds, name) && !is_in_cmdlist(&other_cmds, name)) {\n \t\tfprintf(stderr, _(\"Could not find merge strategy '%s'.\\n\"), name);\n \t\tfprintf(stderr, _(\"Available strategies are:\"));\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 3683c772c5..6d947be858 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -594,14 +594,7 @@ __git_is_configured_remote ()\n \n __git_list_merge_strategies ()\n {\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}'\n+\tgit merge --strategy '?'\n }\n \n __git_merge_strategies=\n-- \n2.16.0.47.g3d9b0fac3a\n\n"},{"id":"337392","messageId":"xmqqo9lhai46.fsf@gitster.mtv.corp.google.com","threadId":"47688","inReplyTo":"20180125094031.6584-1-pclouds@gmail.com","subject":"Re: [PATCH] merge: support --strategy '?' for git-completion.bash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-25T18:49:45Z","receivedAt":"2018-01-25T18:49:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> Bash completion support gets the list of available strategies with a\n> grep and sed trick which does not work on non-C locale since the anchor\n> string is translated and it does not cover custom strategies either.\n>\n> Let's do it a better way and make git-merge provide all available\n> strategies in machine-readable form.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  Another, perhaps better, option is add \"git merge --list-strategies\".\n>  It requires some code movement, so I'll try with a simpler approach\n>  first.\n\nIf you run the probing \"merge -s help\" under C locale, that would\njust be a one-liner, no ;-)  I.e.\n\n> -\tgit merge -s help 2>&1 |\n> +\tLANG=C LC_ALL=C git merge -s help 2>&1 |\n\nNot rejecting the patch, but just wondering.\n\n"},{"id":"337451","messageId":"20180126013142.GA11346@duynguyen.dek-tpc.internal","threadId":"47688","inReplyTo":"xmqqo9lhai46.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] merge: support --strategy '?' for git-completion.bash","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-01-26T01:31:42Z","receivedAt":"2018-01-26T01:32:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jan 25, 2018 at 10:49:45AM -0800, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n> \n> > Bash completion support gets the list of available strategies with a\n> > grep and sed trick which does not work on non-C locale since the anchor\n> > string is translated and it does not cover custom strategies either.\n> >\n> > Let's do it a better way and make git-merge provide all available\n> > strategies in machine-readable form.\n> >\n> > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> > ---\n> >  Another, perhaps better, option is add \"git merge --list-strategies\".\n> >  It requires some code movement, so I'll try with a simpler approach\n> >  first.\n> \n> If you run the probing \"merge -s help\" under C locale, that would\n> just be a one-liner, no ;-)  I.e.\n\nThat was my first choice but I was worried that it failed to catch\ncustom strategies which are preceded with a different anchor string\n\"Available custom strategies are:\".\n\nI didn't look carefully at those sed magic. But it looks like it\ncorrectly handles this case too. So v2 follows below. It still feels\ndirty to do this kind of text manipulation though. But that can wait.\n\n-- 8< --\nSubject: [PATCH] completion: fix completing merge strategies on non-C locales\n\nThe anchor string \"Available strategies are:\" is translatable so\n__git_list_merge_strategies may fail to collect available strategies\nfrom 'git merge' on non-C locales. Force C locale on this command.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n contrib/completion/git-completion.bash | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 3683c772c5..88813e9124 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -594,7 +594,7 @@ __git_is_configured_remote ()\n \n __git_list_merge_strategies ()\n {\n-\tgit merge -s help 2>&1 |\n+\tLANG=C LC_ALL=C git merge -s help 2>&1 |\n \tsed -n -e '/[Aa]vailable strategies are: /,/^$/{\n \t\ts/\\.$//\n \t\ts/.*://\n-- \n2.16.1.196.g4f5118c359\n\n\n\n-- 8< --\n"},{"id":"337491","messageId":"xmqqefmc8qbn.fsf@gitster.mtv.corp.google.com","threadId":"47688","inReplyTo":"20180126013142.GA11346@duynguyen.dek-tpc.internal","subject":"Re: [PATCH] merge: support --strategy '?' for git-completion.bash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-26T17:47:40Z","receivedAt":"2018-01-26T17:47:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> I didn't look carefully at those sed magic. But it looks like it\n> correctly handles this case too. So v2 follows below. It still feels\n> dirty to do this kind of text manipulation though. But that can wait.\n\nI do like \"introduce and use helper feature to produce machine\nparseable text\" when it is appropriate.  But this alternative looks\nmore in line with the current design of completion script and can\neasily be a maint material as pure bugfix for non C locale folks.\n\nWill queue.\n\n> -- 8< --\n> Subject: [PATCH] completion: fix completing merge strategies on non-C locales\n>\n> The anchor string \"Available strategies are:\" is translatable so\n> __git_list_merge_strategies may fail to collect available strategies\n> from 'git merge' on non-C locales. Force C locale on this command.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n\n\n>  contrib/completion/git-completion.bash | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 3683c772c5..88813e9124 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -594,7 +594,7 @@ __git_is_configured_remote ()\n>  \n>  __git_list_merge_strategies ()\n>  {\n> -\tgit merge -s help 2>&1 |\n> +\tLANG=C LC_ALL=C git merge -s help 2>&1 |\n>  \tsed -n -e '/[Aa]vailable strategies are: /,/^$/{\n>  \t\ts/\\.$//\n>  \t\ts/.*://\n"}]}