On 10/13/2016 03:43 PM, Philippe Pepiot wrote:


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.pep...@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 ?

Hum, the smaller improvement I can see is to add this explanation as an inline comment. I think we should also clean up the dorecord return to something better, but we are probably too late in the cycle for that.

Cheers,

--
Pierre-Yves David
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to