[PATCH V2] subrepo: append subrepo path to subrepo push error messages

[Laurens Holst]

Op 16-12-12 23:43, Angel Ezquerra schreef:
> On Sun, Dec 16, 2012 at 2:57 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
>> Angel Ezquerra wrote, On 12/16/2012 01:29 AM:
>>
>>> # HG changeset patch
>>> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>>> # Date 1355438273 -3600
>>> # Node ID cec32425a026c4aeb9a6495086b1a1fbe916be24
>>> # Parent  34a1a639d8358e43f4bcba7b0cff19f4e4e6958d
>>> subrepo: append subrepo path to subrepo push error messages
>>>
>>> This change appends the subrepo path to subrepo push errors. That is, when
>>> there
>>> is an error pushing a subrepo, rather than displaying:
>>>
>>> pushing subrepo MYSUBREPO to PATH
>>> searching for changes
>>> abort: push creates new remote head HEADHASH!
>>> hint: did you forget to merge? use push -f to force
>>>
>>> mercurial will show:
>>>
>>> pushing subrepo MYSUBREPO to PATH
>>> searching for changes
>>> abort: push creates new remote head HEADHASH! (on subrepo MYSUBREPO)
>>> hint: did you forget to merge? use push -f to force
>>>
>>> The rationale for this change is that the current error messages make it
>>> hard
>>> for TortoiseHg (and similar tools) to tell the user which subrepo caused
>>> the
>>> push failure.
>>>
>>> Note that I have also updated test-subrepo.t to reflect this change. The
>>> test
>>> passes on windows.
>>>
>>> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
>>> --- a/mercurial/subrepo.py
>>> +++ b/mercurial/subrepo.py
>>> @@ -567,7 +567,12 @@
>>>            self._repo.ui.status(_('pushing subrepo %s to %s\n') %
>>>                (subrelpath(self), dsturl))
>>>            other = hg.peer(self._repo, {'ssh': ssh}, dsturl)
>>> -        return self._repo.push(other, force, newbranch=newbranch)
>>> +        try:
>>> +            res = self._repo.push(other, force, newbranch=newbranch)
>>> +        except error.Abort, ex:
>>> +            errormsg = ex.message + (' (on subrepo %s)' %
>>> subrelpath(self))
>>
>> The message should be marked up for localization.
> You are right. I believe that it would not be enough to put a "_" call
> around the ' (on subrepo %s)' string, because it would not be found,
> right? Instead I must do:
>
>      errormsg = _('%s (on subrepo %s)' % (ex.message, subrelpath(self)))
>
> Is that correct?

No, it’s should be the other way around:

errormsg = _('%s (on subrepo %s)') % (ex.message, subrelpath(self))

The replacement has to be done outside the localisation call, otherwise 
it will look up "blah (on subrepo x)" in the dictionary and fail to find it.

But using %s to include the message is indeed better than string 
concatenation.

~Laurens