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

[Angel Ezquerra]

On Tue, Dec 18, 2012 at 11:58 AM, Laurens Holst <laurens.nospam at grauw.nl> wrote:
> 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
>

Thank you Laurens,

I'll keep that in mind when I make the next version of this patch.

Cheers,

Angel