| Submitter | Pierre-Yves David |
|---|---|
| Date | June 18, 2017, 6:55 p.m. |
| Message ID | <5e988d4f834b981d8aaf.1497812111@nodosa.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/21482/ |
| State | Accepted |
| Headers | show |
Comments
On 06/18/2017 08:55 PM, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@octobus.net> > # Date 1497696671 -7200 > # Sat Jun 17 12:51:11 2017 +0200 > # Node ID 5e988d4f834b981d8aaf9d77019550d3800687d5 > # Parent 29558247b00eff8c95c7604032b59cfbab34010d > # EXP-Topic config.register > # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ > # hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 5e988d4f834b > config: explicitly track the use of the standard default value > > We introduce a small object used to detect that no specific default value has > been passed to 'ui.config'. We need this explicit special value since "None" is > a valid and common default value. > > The end goal here is to make progress on a centralised and explicit declaration > of the available config option. A first good usecase for this are "default" > value. Before starting looking further down this alley we needs to rework the > handling of default value in the 'ui' object to have all configxyz methods going > through the same logic. This is the first changeset on this trek. The rest of the series (introducing the config registry) is visible here: https://www.mercurial-scm.org/repo/users/marmoute/mercurial/log?revcount=24&rev=5e988d4f834b%3A%3A8451ff2a0ec5 It does the minimal amount of work to: * introduce a central registry, * support reading config item default value from the registry, * test it for all type of config that used a default, * allow extension to extend it (cleanly), * devel warning to catch usage error. From here, we will be able to: * add new types of feature to the registry (alias, documentation, etc) * register all known options (and add support for the necessary special case)
On Sun, Jun 18, 2017 at 11:55 AM, Pierre-Yves David < pierre-yves.david@ens-lyon.org> wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@octobus.net> > # Date 1497696671 -7200 > # Sat Jun 17 12:51:11 2017 +0200 > # Node ID 5e988d4f834b981d8aaf9d77019550d3800687d5 > # Parent 29558247b00eff8c95c7604032b59cfbab34010d > # EXP-Topic config.register > # Available At https://www.mercurial-scm.org/ > repo/users/marmoute/mercurial/ > # hg pull https://www.mercurial-scm.org/ > repo/users/marmoute/mercurial/ -r 5e988d4f834b > config: explicitly track the use of the standard default value > Queued this series because it seems like a strict improvement in default value detection. FWIW I think it is a bit weird to be returning None and then checking "is None" in callers. I think it would be slightly better to factor the low-level config retrieval logic into a function that returns a 2-tuple of (value, is_set) and then call that instead of ui.config(). But this series gets essentially the same result. So meh. I haven't looked at the unpublished changesets yet. But I like the idea of a registry. Even though it seems like over-engineering, it solves a lot of problems around documentation, type checking, registering default values, registering values for different "profiles," associating command arguments with config options, etc. Basically it makes a lot of problems we talked about at the 4.2 Sprint w.r.t. config management feel a lot easier when you can hang additional metadata off config options, which is what a registry will provide. > > We introduce a small object used to detect that no specific default value > has > been passed to 'ui.config'. We need this explicit special value since > "None" is > a valid and common default value. > > The end goal here is to make progress on a centralised and explicit > declaration > of the available config option. A first good usecase for this are "default" > value. Before starting looking further down this alley we needs to rework > the > handling of default value in the 'ui' object to have all configxyz methods > going > through the same logic. This is the first changeset on this trek. > > diff --git a/mercurial/ui.py b/mercurial/ui.py > --- a/mercurial/ui.py > +++ b/mercurial/ui.py > @@ -140,6 +140,10 @@ class httppasswordmgrdbproxy(object): > def _catchterm(*args): > raise error.SignalInterrupt > > +# unique object used to detect no default value has been provided when > +# retrieving configuration value. > +_unset = object() > + > class ui(object): > def __init__(self, src=None): > """Create a fresh new ui object if no src given > @@ -394,7 +398,9 @@ class ui(object): > def configsource(self, section, name, untrusted=False): > return self._data(untrusted).source(section, name) > > - def config(self, section, name, default=None, untrusted=False): > + def config(self, section, name, default=_unset, untrusted=False): > + if default is _unset: > + default = None > if isinstance(name, list): > alternates = name > else: >
Patch
diff --git a/mercurial/ui.py b/mercurial/ui.py --- a/mercurial/ui.py +++ b/mercurial/ui.py @@ -140,6 +140,10 @@ class httppasswordmgrdbproxy(object): def _catchterm(*args): raise error.SignalInterrupt +# unique object used to detect no default value has been provided when +# retrieving configuration value. +_unset = object() + class ui(object): def __init__(self, src=None): """Create a fresh new ui object if no src given @@ -394,7 +398,9 @@ class ui(object): def configsource(self, section, name, untrusted=False): return self._data(untrusted).source(section, name) - def config(self, section, name, default=None, untrusted=False): + def config(self, section, name, default=_unset, untrusted=False): + if default is _unset: + default = None if isinstance(name, list): alternates = name else: