Excerpts from David Soria Parra's message of 2017-03-29 15:25:26 -0700: > On Mon, Mar 27, 2017 at 11:38:06AM -0700, Jun Wu wrote: > > # HG changeset patch > > # User Jun Wu <qu...@fb.com> > > # Date 1490635856 25200 > > # Mon Mar 27 10:30:56 2017 -0700 > > # Node ID 4eb7c76340791f379a34f9df4ec42e0c8b9b2a2f > > # Parent 4a8d065bbad80d3b3401010375bc80165404aa87 > > RFC: implement immutable config objects > > I like the overall approach and that it's parallel to config.config() and > ui.config. We might in later patches combine config.config into this.
If we can replace "x.setconfig(...)" to "x = x.setconfig(...)", and make "config.config" immutable too, the path is worth a try. Practically, that may lead to BCs. > > + super(atomicconfig, self).__init__(title) > > + self._sections = util.sortdict() > Can we use collections.defaultdict(util.sortdict) here? Good advice. > > + > > + def subconfigs(self, section=None): > I am not sure about this API. We already have getsection() from > abstractimmutableconfig. I think this method does two things depending > on the arguments. It was "subconfigs(self)", the "section" was added for performance. I'll revisit if it's still needed. I think eventually we want "debugconfig / config" to be able to get the layered information too. So it can show "config X" set by "file Y" is shallowed by "file Z". Or it can draw a tree. Initially, I was thinking this like a React DOM, a diff algorithm to compare the DOM tree, and replace elements in it. That makes "subconfigs" useful. Later I decided the diff algorithm is a overkill. The same reason applies to why I make "title" a required parameter. So things like "get-subconfig-by-title" is possible. Since we now control the subconfigs directly and build the root config on demand, we never need to get subconfigs from the root config, so the API is unnecessary. I'll make them private in the next version. > > + items = util.sortdict() > > + for subconfig in subconfigs: > > + subconfigitems = subconfig.getsection(section).items() > Could we do an `items.update(subconfig.getsection(section))`? Sure. > > + def subconfigs(self, section=None): > I think this is actually not needed except for dumpconfig, which might just > be a > helper that knows about the internals. > > > + return (self._subconfig,) > > + > > + def sections(self): > > + return self._subconfig.sections() > > + > > + def getsection(self, section): > > + if section not in self._filters: > > + return self._subconfig.getsection(section) > > + items = self._cachedsections.get(section, None) > > + if items is None: > > + filter = self._filters[section] > > + if util.safehasattr(filter, '__call__'): > this should be `callable` > > > + items = filter(self._subconfig.getsection(section)) > > + else: > > + items = filter > > + self._cachedsections[section] = items > > + return items _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
