[PATCH V2] transaction: allow running file generators after finalizers

[FUJIWARA Katsunori]

At Sat, 26 Mar 2016 22:15:50 -0700,
Pierre-Yves David wrote:
> 
> On 03/20/2016 10:49 AM, Durham Goode wrote:
> > # HG changeset patch
> > # User Durham Goode <durham at fb.com>
> > # Date 1458495989 25200
> > #      Sun Mar 20 10:46:29 2016 -0700
> > # Node ID 1431f0447a6b0be851f609b386fabf83b2f54666
> > # Parent  ed75909c4c670a7d9db4a2bef9817a0d5f0b4d9c
> > transaction: allow running file generators after finalizers

[...]

> > […]
> > @@ -276,12 +284,17 @@ class transaction(object):
> >           # but for bookmarks that are handled outside this mechanism.
> >           self._filegenerators[genid] = (order, filenames, genfunc, location)
> >
> > -    def _generatefiles(self, suffix=''):
> > +    def _generatefiles(self, postfinalize=False, suffix=''):
> 
> The postfinalize=False as the default is an issue here,
> 
> Before this patch tr._generatefiles() would get all file generated. 
> After that patch, only part of them are. Third parties and future 
> callers are going to get this wrong and introduce various transaction 
> bug. This is highlighted by the fact they had to be a special case when 
> 'suffix' is passed.
> 
> I think we need three values here: 'all', 'precl', 'postcl':
> 
>    def _generatefiles(self, suffix='', group='all')

If possible, wording without abbreviation is friendly to
non-English-native developer, like me :-)

If "preclose"/"postclose" is long, how about "closing"/"closed" ?


> This is a small change but I think this is a critical enough code that 
> it deserved to be expurged from surprise.
> 
> 
> Also, we should try to avoid changing the position of arguments, this 
> gratuitously break other callers.
> 
> >           # write files registered for generation
> >           any = False
> > -        for entry in sorted(self._filegenerators.values()):
> > +        for id, entry in sorted(self._filegenerators.iteritems()):
> >               any = True
> >               order, filenames, genfunc, location = entry
> > +
> > +            # for generation at closing, check if it's before or after finalize
> > +            if not suffix and (id in postfinalizegenerators) != (postfinalize):
> > +                continue
> > +
> 
> [above, the suffix "hack" I was referring too]
> 
> > […]
> 
> Cheers,
> 
> -- 
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp