[1,of,2,RFC] RFC: implement immutable config objects

Jun Wu quark at fb.com
Sat Apr 1 18:56:25 EDT 2017


Excerpts from Yuya Nishihara's message of 2017-04-01 23:39:34 +0900:
> Yea, the idea sounds good and the next patch proves that this actually works.
> Nice.

Thanks.

> 
> Random nitpicks in line.
> 
> > > +                    yield (section, item, (value, source))
> 
> We might want to keep filename and line number separately in future version.

Since there are other sources like "setconfig", "--config", "--verbose", and
the new "$ENVNAME", maybe "source" needs to a tuple or something more
complex / typed. But it's probably another series.

The immutable configs (other than fileconfig) do not assume the format of
the tuple "(section, item, value)". So if we do change it to "(section,
item, (value, ('file', path, line)))", other objects won't need change.

> > > +class filteredconfig(abstractimmutableconfig):
> > > +    """immutable config that changes other configs"""
> > > +
> > > +    def __init__(self, title, subconfig, filters=None):
> > > +        """
> > > +        filters: a dict-ish, {section: filterfunc or sortdict-ish}
> > > +                 a filterfunc takes sortdict-ish and returns sortdict-ish
> > > +                 a sortdict-ish will replace the section directly
> > > +        """
> 
> So this is more like "map" operation? I thought filterfunc would return
> booleans.

It should probably be renamed to "transformfunc".

> > > +            if util.safehasattr(filter, '__call__'):
> > this should be `callable`
> > 
> > > +                items = filter(self._subconfig.getsection(section))
> > > +            else:
> > > +                items = filter
> 
> Do we really need to overload the type of filter?

Good point. The "transform" function could take a section:

    def transform(section, items):
        ....

So the "static items" type is no longer needed.

I wrote that mainly for performance consideration, and it seems unnecessary.


More information about the Mercurial-devel mailing list