[PATCH 1 of 4 V2] flagutil: introduce a flagprocessorsmixin class

Augie Fackler raf at durin42.com
Thu Sep 5 14:37:00 EDT 2019


On Thu, Sep 05, 2019 at 10:24:25AM +0200, Pierre-Yves David wrote:
>
>
> On 9/5/19 1:43 AM, Yuya Nishihara wrote:
> > On Mon, 2 Sep 2019 15:03:00 +0200, Pierre-Yves David wrote:
> > > On 8/31/19 5:02 AM, Yuya Nishihara wrote:
> > > > On Sun, 11 Aug 2019 13:10:47 +0200, Pierre-Yves David wrote:
> > > > > # HG changeset patch
> > > > > # User Pierre-Yves David <pierre-yves.david at octobus.net>
> > > > > # Date 1565219568 -7200
> > > > > #      Thu Aug 08 01:12:48 2019 +0200
> > > > > # Node ID e6d22ec889461dbe819e0dbf4d2f0167736b2416
> > > > > # Parent  6d61be152c5515fed315885f6c9cf9defe73de71
> > > > > # EXP-Topic flag-processors
> > > > > # Available At https://bitbucket.org/octobus/mercurial-devel/
> > > > > #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r e6d22ec88946
> > > > > flagutil: introduce a flagprocessorsmixin class
> > > >
> > > > CC +indygreg
> > > >
> > > > > diff --git a/mercurial/revlogutils/flagutil.py b/mercurial/revlogutils/flagutil.py
> > > > > --- a/mercurial/revlogutils/flagutil.py
> > > > > +++ b/mercurial/revlogutils/flagutil.py
> > > > > @@ -78,3 +78,74 @@ def insertflagprocessor(flag, processor,
> > > > >            msg = _("cannot register multiple processors on flag '%#x'.") % (flag)
> > > > >            raise error.Abort(msg)
> > > > >        flagprocessors[flag] = processor
> > > > > +
> > > > > +class flagprocessorsmixin(object):
> > > > > +    """basic mixin to support revlog flag processing
> > > > > +
> > > > > +    Make sure the `_flagprocessors` attribute is set at ``__init__`` time.
> > > >
> > > > I feel a plain function taking (revlog, flagprocessors, ...) is simpler
> > > > than the mixin class which implicitly depends on revlog attributes. But
> > > > maybe I'm biased. I don't like class inheritance in general.
> > >
> > > Are you suggesting (revlog, currently_supported_flags, …)? If so, I am
> > > afraid this would lead to wrong API usage.
> >
> > Probably yes.
> >
> > > It is important that the
> > > revlog bear the supported flags to help enforcing extension/config
> > > isolation.
> >
> > Anyway, the user of the mixin would have to set self._flagprocessors
> > properly. I don't see much difference between mixin + attributes and
> > passing it as a function argument.
>
> I am afraid people would simply call `(revlog, flagutils.flagprocessors,
> ...)` instead of `(revlog, revlog._flagprocessors, ...)`.
>
> Do you want me to switch to the function + attribute approach ?

I'd be more comfortable with that, yeah. Mention the "don't call this
with flagutils.flagprocesors" requirement in the docstring with the
rationale (IIRC it's because not all flag processors will be enabled
for all repos.)

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


More information about the Mercurial-devel mailing list