Re: [PATCH] merge: support --strategy '?' for git-completion.bash

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 26, 2018, 17:47 UTC
Message-ID: <xmqqefmc8qbn.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20180126013142.GA11346@duynguyen.dek-tpc.internal>

Duy Nguyen <pclouds@gmail.com> writes:

> I didn't look carefully at those sed magic. But it looks like it
> correctly handles this case too. So v2 follows below. It still feels
> dirty to do this kind of text manipulation though. But that can wait.

I do like "introduce and use helper feature to produce machine parseable text" when it is appropriate. But this alternative looks more in line with the current design of completion script and can easily be a maint material as pure bugfix for non C locale folks.

Will queue.

Show 9 quoted lines

> -- 8< --
> Subject: [PATCH] completion: fix completing merge strategies on non-C locales
>
> The anchor string "Available strategies are:" is translatable so
> __git_list_merge_strategies may fail to collect available strategies
> from 'git merge' on non-C locales. Force C locale on this command.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

Show 16 quoted lines

>  contrib/completion/git-completion.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 3683c772c5..88813e9124 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -594,7 +594,7 @@ __git_is_configured_remote ()
>  
>  __git_list_merge_strategies ()
>  {
> -	git merge -s help 2>&1 |
> +	LANG=C LC_ALL=C git merge -s help 2>&1 |
>  	sed -n -e '/[Aa]vailable strategies are: /,/^$/{
>  		s/\.$//
>  		s/.*://