[PATCH STABLE] bundle2: localize "remote: "

[Gregory Szorc]

On Tue, Aug 30, 2016 at 2:44 PM, Matt Mackall <mpm at selenic.com> wrote:

> On Tue, 2016-08-30 at 13:33 -0700, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc at gmail.com>
> > # Date 1472588883 25200
> > #      Tue Aug 30 13:28:03 2016 -0700
> > # Branch stable
> > # Node ID f7ec2b2f002750a7748dc5d0375035e181d1c3b0
> > # Parent  9a9629b9416c262ff62bbd17568fcd2a2e09b4be
> > bundle2: localize "remote: "
> >
> > We localize "remote: " everywhere else. This unlocalized use was
> > introduced in b7435117d951.
>
> Ok, this is slightly more informative than Akihiko's patch.. but I'm still
> not
> happy:
>
> > -        op.ui.status(('remote: %s\n' % line))
>
> See those extra parens? They were put there to defeat check-code, which
> reminds
> you to use _() in the first place. That is a signal to me that there was
> intent
> for this to NOT be translated.
>
> If you don't even mention the parens, I have to assume you didn't notice
> them
> and didn't consider the possibility that there was a reason for them.
>

You are correct about me not recognizing that the parens were there to
defeat check-code.

However, I'm going to posit that _() not being used was accidental. We
translate "remote: " everywhere else. This code is running locally (not on
the server). So I don't see why we wouldn't translate.


>
> > +        op.ui.status('%s%s\n' % (_('remote: '), line))
>
> I'm not sure why you're doing the %s%s thing. It's more complex than
> necessary
> and the existing string literal is already translated elsewhere.
>

That's fair.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20160830/01af640d/attachment.html>