Subject: Re: [PATCH v2 1/1] ref-filter: sort detached HEAD lines firstly

From: Matthew DeVore <[email-masked]>
Date: Wed, 12 Jun 2019 14:21:11 -0700

On Wed, Jun 12, 2019 at 02:09:53PM -0700, Junio C Hamano wrote:
> Junio C Hamano <[email-masked]> writes:
> 
> >> +		/*
> >> +		 * When sorting by name, we should put "detached" head lines,
> >> +		 * which are all the lines in parenthesis, before all others.
> >> +		 * This usually is automatic, since "(" is before "refs/" and
> >> +		 * "remotes/", but this does not hold for zh_CN, which uses
> >> +		 * full-width parenthesis, so make the ordering explicit.
> >> +		 */
> >> +		if (a_detached != (b->kind & FILTER_REFS_DETACHED_HEAD))
> >> +			cmp = a_detached ? -1 : 1;
> >
> > So, comparing a detached and an undetached ones, the detached side
> > always sorts lower.  Good.  And ...
> >
> >> +		else
> >> +			cmp = cmp_fn(va->s, vb->s);
> >
> > ... otherwise we compare the string using the given function.
> >
> > Sounds sensible.  Will queue.
> 
> Stepping back a bit, why are we even allowing the surrounding ()
> pair to be futzed by the translators?

I was thinking about removing () from the translated strings, but decided
against it since there are a lot of full-width parenthesis in the translated
strings already:

$ cd po; git grep -B 1 'msgstr.*（'
... 246 matches in zh_CN ...

and it seems strange to force only a few pairs of parens to be half-width to
make the code simpler. I don't know if that's a great argument, since it is
somewhat aesthetic. I would have liked half-width parens more if it were
closing off purely ASCII text. But it is in fact surrounding Chinese text:

$ git branch
* （头指针分离于 cf0246a5cc）

> 
> IOW, shouldn't our code more like this from the beginning, with or
> without Chinese translation?
> 
> With a bit more work, we may even be able to lose "make sure this
> matches the one in wt-status.c" comment as losing the leading '('
> would take us one step closer to have an identical string here as we
> have in wt-status.c
> 
>  ref-filter.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/ref-filter.c b/ref-filter.c
> index 8500671bc6..7e4705fcb2 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1459,20 +1459,22 @@ char *get_head_description(void)
>  		strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
>  			    state.branch);
>  	else if (state.detached_from) {
> +		strbuf_addch(&desc, '(');
>  		if (state.detached_at)
>  			/*
>  			 * TRANSLATORS: make sure this matches "HEAD
>  			 * detached at " in wt-status.c
>  			 */
> -			strbuf_addf(&desc, _("(HEAD detached at %s)"),
> -				state.detached_from);
> +			strbuf_addf(&desc, _("HEAD detached at %s"),
> +				    state.detached_from);
>  		else
>  			/*
>  			 * TRANSLATORS: make sure this matches "HEAD
>  			 * detached from " in wt-status.c
>  			 */
> -			strbuf_addf(&desc, _("(HEAD detached from %s)"),
> +			strbuf_addf(&desc, _("HEAD detached from %s"),
>  				state.detached_from);
> +		strbuf_addch(&desc, ')');
>  	}
>  	else
>  		strbuf_addstr(&desc, _("(no branch)"));
> 
>