[PATCH 07 of 11 V2] ui: use scmutil.rccomponents to load configs (BC)

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


On 3/22/17 7:50 AM, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark at fb.com>
> # Date 1489462886 25200
> #      Mon Mar 13 20:41:26 2017 -0700
> # Node ID add83f47bf3a51edbd58aa0fb6e571d186bdae6e
> # Parent  df768455486ff51b4047fd3a8dfbef158516d2aa
> ui: use scmutil.rccomponents to load configs (BC)
>
> This is BC because system configs won't be able to override $EDITOR, $PAGER.
> The new behavior is arguably more rational.
>
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -212,6 +212,18 @@ class ui(object):
>           u = cls()
>           # we always trust global config files
> -        for f in scmutil.rcpath():
> -            u.readconfig(f, trust=True)
> +        for (t, f) in scmutil.rccomponents():
> +            if t == 'path':
> +                u.readconfig(f, trust=True)
> +            elif t == 'items':
> +                sections = set()
> +                for section, name, value, source in f:
> +                    # do not set ocfg
> +                    u._tcfg.set(section, name, value, source)
> +                    u._ucfg.set(section, name, value, source)
> +                    sections.add(section)
> +                for section in sections:
> +                    u.fixconfig(section=section)
> +            else:
> +                raise error.ProgrammingError('unexpected rccomponent: %s' % t)
>           return u

Seeing this makes me not like this "union type" (rccomponents) that 
you've created. Can we do this differently for more cleanliness?

Consider instead of having rccomponents be a parsed config "source" 
(either a file or environment for now). Then this patch simply combines 
them in order. This gives us the nice property of reading each config 
file only once, which was a patch series that dsop sent but had BC 
issues, but doing it this way means there are no BC issues. Of course, 
it also cleans up this code so we don't have the type check in there.

Essentially, envconfig() can be generalized into a "parsedconfig" 
container, and then it's just about ordering for the final config from 
there.

I think this would make the code cleaner -- thoughts?

>   
> diff --git a/tests/test-config-env.py b/tests/test-config-env.py
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-config-env.py
> @@ -0,0 +1,48 @@
> +# Test the config layer generated by environment variables
> +
> +from __future__ import absolute_import, print_function
> +
> +import os
> +
> +from mercurial import (
> +    encoding,
> +    scmutil,
> +    ui as uimod,
> +)
> +
> +testtmp = encoding.environ['TESTTMP']
> +
> +# prepare hgrc files
> +def join(name):
> +    return os.path.join(testtmp, name)
> +
> +with open(join('sysrc'), 'w') as f:
> +    f.write('[ui]\neditor=e0\n[pager]\npager=p0\n')
> +
> +with open(join('userrc'), 'w') as f:
> +    f.write('[ui]\neditor=e1')
> +
> +# replace rcpath functions so they point to the files above
> +def systemrcpath():
> +    return [join('sysrc')]
> +
> +def userrcpath():
> +    return [join('userrc')]
> +
> +scmutil.systemrcpath = systemrcpath
> +scmutil.userrcpath = userrcpath
> +os.path.isdir = lambda x: False # hack: do not load default.d/*.rc
> +
> +# utility to print configs
> +def printconfigs(env):
> +    encoding.environ = env
> +    scmutil._rccomponents = None # reset cache
> +    ui = uimod.ui.load()
> +    for section, name, value in ui.walkconfig():
> +        source = ui.configsource(section, name)
> +        print('%s.%s=%s # %s' % (section, name, value, source))
> +    print('')
> +
> +# environment variable overrides
> +printconfigs({})
> +printconfigs({'EDITOR': 'e2', 'PAGER': 'p2'})
> diff --git a/tests/test-config-env.py.out b/tests/test-config-env.py.out
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-config-env.py.out
> @@ -0,0 +1,6 @@
> +pager.pager=p0 # $TESTTMP/sysrc:4
> +ui.editor=e1 # $TESTTMP/userrc:2
> +
> +pager.pager=p2 # $PAGER
> +ui.editor=e1 # $TESTTMP/userrc:2
> +
> diff --git a/tests/test-config.t b/tests/test-config.t
> --- a/tests/test-config.t
> +++ b/tests/test-config.t
> @@ -165,2 +165,16 @@ edit failure
>     abort: edit failed: false exited with status 1
>     [255]
> +
> +config affected by environment variables
> +
> +  $ EDITOR=e1 VISUAL=e2 hg config --debug | grep 'ui\.editor'
> +  $VISUAL: ui.editor=e2
> +
> +  $ VISUAL=e2 hg config --debug --config ui.editor=e3 | grep 'ui\.editor'
> +  --config: ui.editor=e3
> +
> +  $ PAGER=p1 hg config --debug | grep 'pager\.pager'
> +  $PAGER: pager.pager=p1
> +
> +  $ PAGER=p1 hg config --debug --config pager.pager=p2 | grep 'pager\.pager'
> +  --config: pager.pager=p2
>



More information about the Mercurial-devel mailing list