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

Pierre-Yves David pierre-yves.david at ens-lyon.org
Thu Sep 5 04:24:25 EDT 2019



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 ?


-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list