[PATCH 04 of 10] scmutil: add a rcpath-like method to return multiple config sources

Jun Wu quark at fb.com
Tue Mar 14 00:44:18 EDT 2017


Excerpts from David Soria Parra's message of 2017-03-13 21:37:57 -0700:
> On Mon, Mar 13, 2017 at 08:44:49PM -0700, Jun Wu wrote:
> > scmutil: add a rcpath-like method to return multiple config sources
> 
> nice series :)
> 
> > +def rccomponents():
> > +    '''return an ordered [(type, obj)] about where to load configs.
> > +
> > +    respect $HGRCPATH. if $HGRCPATH is empty, only .hg/hgrc of current repo is
> > +    used. if $HGRCPATH is not set, the platform default will be used.
> > +
> > +    if a directory is provided, *.rc files under it will be used.
> > +
> > +    type could be either 'path' or 'items', if type is 'path', obj is a string,
> > +    and is the config file path. if type is 'items', obj is a list of (section,
> > +    name, value, source) that should fill the config directly.
> > +    '''
> > +    global _rccomponents
> > +    if _rccomponents is None:
> > +        if 'HGRCPATH' in encoding.environ:
> > +            # assume HGRCPATH is all about user configs so environments, so
> > +            # environments can be overrided.
> I think this should be 'environments can be 'overridden'. You duplicated 'so
> environments'.

Good catch. My Vim was behaving weird recently.

> 
> > +            _rccomponents = [('items', envconfig(_sysenvlist))]
> > +            for p in encoding.environ['HGRCPATH'].split(pycompat.ospathsep):
> > +                if not p:
> > +                    continue
> > +                p = util.expandpath(p)
> Just a nit, maybe move this into a helper function?

Good advice. The code was copy-pasted from rcpath()

> def readpath(p):
>     p = util.expandpath(p)
>     if os.path.isdir(p):
>         entries = [(f,k) for (f,k) in osutil.listdir(p) if f.endswith('.rc')]
>         for f, kind in entries:
>             yield ('path', os.path.join(p,f))
>     else
>         return ('path', p)
> 
> _rccomponents.extend(readpath(p))
> > +                if os.path.isdir(p):
> > +                    for f, kind in osutil.listdir(p):
> > +                        if f.endswith('.rc'):
> > +                            _rccomponents.append(('path', os.path.join(p, f)))
> > +                else:
> > +                    _rccomponents.append(('path', p))
> > +        else:
> > +            syspath, userpath = osrcpath()
> > +
> > +            def pathize(path):
> > +                return ('path', path)
> > +
> > +            _rccomponents = map(pathize, syspath)
> > +            _rccomponents.append(('items', envconfig(_sysenvlist)))
> > +            _rccomponents.extend(map(pathize, userpath))
> 
> I see we barely use map and prefer list-comprehensions. I don't mind either, i
> am just not sure if we have a coding style around it.

I don't feel strong. The "map" version seems to be easier to read in this
case.


More information about the Mercurial-devel mailing list