[PATCH] bundle2: handleoutput i18n

[Akihiko Odaki]

Hi,

Thanl you for reviewing my patch.

On 2016-08-30 09:38, Matt Mackall wrote:
 > On Sun, 2016-08-28 at 10:57 +0900, Akihiko Odaki wrote:
 >> # HG changeset patch
 >> # User Akihiko Odaki <akihiko.odaki.4i at stu.hosei.ac.jp>
 >> # Date 1472348629 -32400
 >> #      Sun Aug 28 10:43:49 2016 +0900
 >> # Node ID 3e9c35285720b3eecbda43d3d60e483d63ba7ae4
 >> # Parent  4fca69e3d51cc45f331a74caa6e923523ebf7f02
 >> bundle2: handleoutput i18n
 >
 > This would be easier to read as "bundle2: localize handleoutput 
remote prompts"

I understand.

 >
 >> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
 >> --- a/mercurial/bundle2.py
 >> +++ b/mercurial/bundle2.py
 >> @@ -1467,7 +1467,7 @@ def handlecheckheads(op, inpart):
 >>  def handleoutput(op, inpart):
 >>      """forward output captured on the server to the client"""
 >>      for line in inpart.read().splitlines():
 >> -        op.ui.status(('remote: %s\n' % line))
 >
 > You'll note there is an extra level of useless parentheses here. They 
actually
 > serve a purpose: they stop check-code.py from complaining about not 
using _().
 >
 > So someone didn't put _() there on purpose. Whenever you see something
 > apparently pointless that was also clearly done on purpose, you need 
to figure
 > out why it was done before you take it out: you might be breaking 
something that
 > isn't obvious.

I thought it should use _() because other codes do so. Here is `grep -r 
'remote: ' mercurial`.

mercurial/commands.py:            ui.write(_('remote: %s\n') % (', 
'.join(t)))
mercurial/commands.py:            ui.status(_('remote: (synced)\n'))
mercurial/exchange.py:            pushop.ui.status(_('remote: %s\n') % exc)
mercurial/phases.py:                               ' from remote: %s\n') 
% nhex)
mercurial/phases.py:            repo.ui.warn(_('ignoring unexpected root 
from remote: %i %s\n')
mercurial/wireproto.py:            self.ui.status(_('remote: '), l)
mercurial/wireproto.py:                self.ui.status(_('remote: '), l)
mercurial/merge.py:    repo.ui.debug(" ancestor: %s, local: %s, remote: 
%s\n" % (pa, wctx, p2))
mercurial/sshpeer.py:            ui.status(_("remote: "), l, '\n')
mercurial/sshpeer.py:                self.ui.debug("remote: ", l)
mercurial/sshpeer.py:                self.ui.status(_("remote: "), l)
mercurial/bundle2.py:        op.ui.status(('remote: %s\n' % line))

If it is intentional, however, I would like to ask the reason.

 >
 > But if you look into it and discover there was indeed no good reason 
for it, you
 > should explain that in your commit message. Then reviewers will know 
you were
 > paying attention.
 >
 >> +        op.ui.status((_('remote: %s\n') % line))
 >
 > Leaving in the extra parentheses now useless for real.
 >
 > --
 > Mathematics is the supreme nostalgia of our time.
 >

Regards,
Akihiko Odaki