On Thu, Oct 13, 2016 at 04:13:43PM +0200, Pierre-Yves David wrote: > > > 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.
FWIW, I'm fine with this patch as-is, and a comment added as a followup since it's a little wonky. It's definitely too late in the cycle to go monkeying with dorecord's interface. > > Cheers, > > -- > Pierre-Yves David > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel