On Wed, Jun 12, 2019 at 02:09:53PM -0700, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> 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)")); > >