[PATCH 1 of 2] commit: return 1 for interactive commit with no changes (issue5397)

[Philippe Pepiot]

On 10/13/2016 03:22 PM, Pierre-Yves David wrote:
>
>
> On 10/13/2016 02:32 PM, Philippe Pepiot wrote:
>> # HG changeset patch
>> # User Philippe Pepiot <philippe.pepiot at logilab.fr>
>> # Date 1476266974 -7200
>> #      Wed Oct 12 12:09:34 2016 +0200
>> # Node ID fa75185b8901e58d4b1117985e9f3f20e89c4e01
>> # Parent  b85fa6bf298be07804a74d8fdec0d19fdbc6d740
>> # EXP-Topic record-return-code
>> commit: return 1 for interactive commit with no changes (issue5397)
>>
>> For consistency with non interactive commit
>>
>> diff --git a/mercurial/commands.py b/mercurial/commands.py
>> --- a/mercurial/commands.py
>> +++ b/mercurial/commands.py
>> @@ -1648,9 +1648,9 @@ def commit(ui, repo, *pats, **opts):
>>  def _docommit(ui, repo, *pats, **opts):
>>      if opts.get('interactive'):
>>          opts.pop('interactive')
>> -        cmdutil.dorecord(ui, repo, commit, None, False,
>> -                        cmdutil.recordfilter, *pats, **opts)
>> -        return
>> +        ret = cmdutil.dorecord(ui, repo, commit, None, False,
>> +                               cmdutil.recordfilter, *pats, **opts)
>> +        return 1 if ret == 0 else ret
>
> I'm confused about this return value. "if ret == 0" we return 1; if 
> return is not zero, we return the value. Do we never return 0 or am I 
> missing something ?

dorecord() return either 0 if there is no changes to record or the 
return of commitfunc passed in arguments. Here commitfunc = commit that 
return 1 or None, here is the trick.

I agree this is confusing (see also how shelve use dorecord), maybe we 
could raise a custom exception instead of returning 0 in dorecord to 
avoid collisions with commitfunc return value, but I'm not sure how this 
could affect 3rd party extensions ?

Cheers,

-- 
Philippe Pepiot
https://www.logilab.fr