[PATCH 05 of 11 V2] scmutil: extract path reading function from rccomponents

Ryan McElroy rm at fb.com
Wed Mar 22 07:30:37 EDT 2017


On 3/22/17 7:50 AM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1490168676 25200
> #      Wed Mar 22 00:44:36 2017 -0700
> # Node ID c18d9f3e0dac6d1676bd58617477c64953be96a3
> # Parent  75f661b31640a914dd513d42b2ce1389c9b61c0a
> # Available At https://urldefense.proofpoint.com/v2/url?u=https-3A__bitbucket.org_quark-2Dzju_hg-2Ddraft&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=4N9cXP1PfAM3QX2l-gLhA58Qj5cP-SbF4XJKyQ5Yjhw&s=WVkqRLpTQLL-QK5ldWoPQFxy4VmwwR7uNpH4piHGljQ&e=
> #              hg pull https://urldefense.proofpoint.com/v2/url?u=https-3A__bitbucket.org_quark-2Dzju_hg-2Ddraft&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=4N9cXP1PfAM3QX2l-gLhA58Qj5cP-SbF4XJKyQ5Yjhw&s=WVkqRLpTQLL-QK5ldWoPQFxy4VmwwR7uNpH4piHGljQ&e=  -r c18d9f3e0dac
> scmutil: extract path reading function from rccomponents
>
> This makes the code better structured. A side effect is "normpath" will be
> called on paths in $HGRCPATH, which seems to be more correct.
>
> diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
> --- a/mercurial/scmutil.py
> +++ b/mercurial/scmutil.py
> @@ -452,4 +452,13 @@ def rcpath():
>   _rccomponents = None
>   
> +def _readrcpath(path):
> +    '''path could be a single file or a directory. return a list of paths'''

Thanks for providing a docblock -- we need more of these in general, but 
the return value is unclear from the doc block (the type is clear, but 
not what it contains). I'd modify it so it's clear that the list 
contains paths of files in the order that they should be read).

> +    p = util.expandpath(path)
> +    join = os.path.join
> +    if os.path.isdir(p):
> +        return [join(p, f) for f, k in osutil.listdir(p) if f.endswith('.rc')]
> +    else:
> +        return [p]
> +
>   def rccomponents():
>       '''return an ordered [(type, obj)] about where to load configs.
> @@ -465,4 +474,8 @@ def rccomponents():
>       '''
>       global _rccomponents
> +
> +    def pathize(path):
> +        return ('path', os.path.normpath(path))
> +

It seems that this patch and the last patch could be re-done to reduce 
the number of churning lines, making both easier to review. For example, 
why not introduce _readrcpath in it's own patch, then use it when you 
create rccomponents? That would make each patch smaller and the overall 
arc of this series more clear to me.

>       if _rccomponents is None:
>           if 'HGRCPATH' in encoding.environ:
> @@ -473,15 +486,6 @@ def rccomponents():
>                   if not p:
>                       continue
> -                p = util.expandpath(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))
> +                _rccomponents.extend(map(pathize, _readrcpath(p)))
>           else:
> -            def pathize(path):
> -                return ('path', os.path.normpath(path))
> -
>               _rccomponents = map(pathize, defaultrcpath())
>               _rccomponents.extend(map(pathize, systemrcpath()))
>



More information about the Mercurial-devel mailing list