Patchwork [01,of,10] config: explicitly track the use of the standard default value

login
register
mail settings
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

Pierre-Yves David - June 18, 2017, 6:55 p.m.
# 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.
Pierre-Yves David - June 18, 2017, 6:59 p.m.
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)
Gregory Szorc - June 21, 2017, 3:22 a.m.
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: