Patchwork [2,of,6] configitems: add an official API for extensions to register config item

login
register
mail settings
Submitter Pierre-Yves David
Date June 27, 2017, 1 p.m.
Message ID <944f8e3191e6fa37f27d.1498568431@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/21772/
State Accepted
Headers show

Comments

Pierre-Yves David - June 27, 2017, 1 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@octobus.net>
# Date 1497700100 -7200
#      Sat Jun 17 13:48:20 2017 +0200
# Node ID 944f8e3191e6fa37f27d52d3c94d1ef702408ae4
# Parent  714ce79885e32305c45febf0da59b2ce2093967d
# 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 944f8e3191e6
configitems: add an official API for extensions to register config item

Extensions can have a 'configtable' mapping and use
'registrar.configitem(table)' to retrieve the registration function.

This behave in the same way as the other way for extensions to register new
items (commands, colors, etc).
Yuya Nishihara - June 29, 2017, 1:48 p.m.
On Tue, 27 Jun 2017 15:00:31 +0200, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> # Date 1497700100 -7200
> #      Sat Jun 17 13:48:20 2017 +0200
> # Node ID 944f8e3191e6fa37f27d52d3c94d1ef702408ae4
> # Parent  714ce79885e32305c45febf0da59b2ce2093967d
> # 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 944f8e3191e6
> configitems: add an official API for extensions to register config item
> 
> Extensions can have a 'configtable' mapping and use
> 'registrar.configitem(table)' to retrieve the registration function.
> 
> This behave in the same way as the other way for extensions to register new
> items (commands, colors, etc).
> 
> diff --git a/mercurial/configitems.py b/mercurial/configitems.py
> --- a/mercurial/configitems.py
> +++ b/mercurial/configitems.py
> @@ -13,6 +13,11 @@ from . import (
>      error,
>  )
>  
> +def loadconfigtable(ui, extname, configtable):
> +    """update config item known to the ui with the extension ones"""
> +    for section, items in  configtable.items():
> +        ui._knownconfig.setdefault(section, {}).update(items)

So this effectively updates the global configitems.coreitems, and that's
unavoidable since the passed ui is a temporary object.

Do you have a plan to make _knownconfig a real local variable?
Pierre-Yves David - June 29, 2017, 1:59 p.m.
On 06/29/2017 03:48 PM, Yuya Nishihara wrote:
> On Tue, 27 Jun 2017 15:00:31 +0200, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@octobus.net>
>> # Date 1497700100 -7200
>> #      Sat Jun 17 13:48:20 2017 +0200
>> # Node ID 944f8e3191e6fa37f27d52d3c94d1ef702408ae4
>> # Parent  714ce79885e32305c45febf0da59b2ce2093967d
>> # 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 944f8e3191e6
>> configitems: add an official API for extensions to register config item
>>
>> Extensions can have a 'configtable' mapping and use
>> 'registrar.configitem(table)' to retrieve the registration function.
>>
>> This behave in the same way as the other way for extensions to register new
>> items (commands, colors, etc).
>>
>> diff --git a/mercurial/configitems.py b/mercurial/configitems.py
>> --- a/mercurial/configitems.py
>> +++ b/mercurial/configitems.py
>> @@ -13,6 +13,11 @@ from . import (
>>       error,
>>   )
>>   
>> +def loadconfigtable(ui, extname, configtable):
>> +    """update config item known to the ui with the extension ones"""
>> +    for section, items in  configtable.items():
>> +        ui._knownconfig.setdefault(section, {}).update(items)
> 
> So this effectively updates the global configitems.coreitems, and that's
> unavoidable since the passed ui is a temporary object.

Hum, yes good catch, we are missing some copying around here. However 
you mention the ui being a temporary object can you elaborate a bit? 
(I've some notion of the weird life cycle of UI object but I'm not sure 
I follow you here.

> Do you have a plan to make _knownconfig a real local variable?

Yes!
Yuya Nishihara - June 29, 2017, 2:11 p.m.
On Thu, 29 Jun 2017 15:59:17 +0200, Pierre-Yves David wrote:
> On 06/29/2017 03:48 PM, Yuya Nishihara wrote:
> > On Tue, 27 Jun 2017 15:00:31 +0200, Pierre-Yves David wrote:
> >> # HG changeset patch
> >> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> >> # Date 1497700100 -7200
> >> #      Sat Jun 17 13:48:20 2017 +0200
> >> # Node ID 944f8e3191e6fa37f27d52d3c94d1ef702408ae4
> >> # Parent  714ce79885e32305c45febf0da59b2ce2093967d
> >> # 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 944f8e3191e6
> >> configitems: add an official API for extensions to register config item
> >>
> >> Extensions can have a 'configtable' mapping and use
> >> 'registrar.configitem(table)' to retrieve the registration function.
> >>
> >> This behave in the same way as the other way for extensions to register new
> >> items (commands, colors, etc).
> >>
> >> diff --git a/mercurial/configitems.py b/mercurial/configitems.py
> >> --- a/mercurial/configitems.py
> >> +++ b/mercurial/configitems.py
> >> @@ -13,6 +13,11 @@ from . import (
> >>       error,
> >>   )
> >>   
> >> +def loadconfigtable(ui, extname, configtable):
> >> +    """update config item known to the ui with the extension ones"""
> >> +    for section, items in  configtable.items():
> >> +        ui._knownconfig.setdefault(section, {}).update(items)
> > 
> > So this effectively updates the global configitems.coreitems, and that's
> > unavoidable since the passed ui is a temporary object.
> 
> Hum, yes good catch, we are missing some copying around here. However 
> you mention the ui being a temporary object can you elaborate a bit? 
> (I've some notion of the weird life cycle of UI object but I'm not sure 
> I follow you here.

extensions.loadall() is called with "lui", which can be a temporary copy of
the main "ui". "lui" exists for preloading repo hgrc in dispatch layer.
Yuya Nishihara - June 29, 2017, 2:29 p.m.
On Thu, 29 Jun 2017 23:11:57 +0900, Yuya Nishihara wrote:
> On Thu, 29 Jun 2017 15:59:17 +0200, Pierre-Yves David wrote:
> > On 06/29/2017 03:48 PM, Yuya Nishihara wrote:
> > > On Tue, 27 Jun 2017 15:00:31 +0200, Pierre-Yves David wrote:
> > >> # HG changeset patch
> > >> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> > >> # Date 1497700100 -7200
> > >> #      Sat Jun 17 13:48:20 2017 +0200
> > >> # Node ID 944f8e3191e6fa37f27d52d3c94d1ef702408ae4
> > >> # Parent  714ce79885e32305c45febf0da59b2ce2093967d
> > >> # 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 944f8e3191e6
> > >> configitems: add an official API for extensions to register config item
> > >>
> > >> Extensions can have a 'configtable' mapping and use
> > >> 'registrar.configitem(table)' to retrieve the registration function.
> > >>
> > >> This behave in the same way as the other way for extensions to register new
> > >> items (commands, colors, etc).
> > >>
> > >> diff --git a/mercurial/configitems.py b/mercurial/configitems.py
> > >> --- a/mercurial/configitems.py
> > >> +++ b/mercurial/configitems.py
> > >> @@ -13,6 +13,11 @@ from . import (
> > >>       error,
> > >>   )
> > >>   
> > >> +def loadconfigtable(ui, extname, configtable):
> > >> +    """update config item known to the ui with the extension ones"""
> > >> +    for section, items in  configtable.items():
> > >> +        ui._knownconfig.setdefault(section, {}).update(items)
> > > 
> > > So this effectively updates the global configitems.coreitems, and that's
> > > unavoidable since the passed ui is a temporary object.
> > 
> > Hum, yes good catch, we are missing some copying around here. However 
> > you mention the ui being a temporary object can you elaborate a bit? 
> > (I've some notion of the weird life cycle of UI object but I'm not sure 
> > I follow you here.
> 
> extensions.loadall() is called with "lui", which can be a temporary copy of
> the main "ui". "lui" exists for preloading repo hgrc in dispatch layer.

Anyway, I think loadconfigtable() needs to update some global table. Otherwise
newly created ui object wouldn't see the defaults defined by loaded extensions.
Pierre-Yves David - June 29, 2017, 2:37 p.m.
On 06/29/2017 04:29 PM, Yuya Nishihara wrote:
> On Thu, 29 Jun 2017 23:11:57 +0900, Yuya Nishihara wrote:
>> On Thu, 29 Jun 2017 15:59:17 +0200, Pierre-Yves David wrote:
>>> On 06/29/2017 03:48 PM, Yuya Nishihara wrote:
>>>> On Tue, 27 Jun 2017 15:00:31 +0200, Pierre-Yves David wrote:
>>>>> # HG changeset patch
>>>>> # User Pierre-Yves David <pierre-yves.david@octobus.net>
>>>>> # Date 1497700100 -7200
>>>>> #      Sat Jun 17 13:48:20 2017 +0200
>>>>> # Node ID 944f8e3191e6fa37f27d52d3c94d1ef702408ae4
>>>>> # Parent  714ce79885e32305c45febf0da59b2ce2093967d
>>>>> # 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 944f8e3191e6
>>>>> configitems: add an official API for extensions to register config item
>>>>>
>>>>> Extensions can have a 'configtable' mapping and use
>>>>> 'registrar.configitem(table)' to retrieve the registration function.
>>>>>
>>>>> This behave in the same way as the other way for extensions to register new
>>>>> items (commands, colors, etc).
>>>>>
>>>>> diff --git a/mercurial/configitems.py b/mercurial/configitems.py
>>>>> --- a/mercurial/configitems.py
>>>>> +++ b/mercurial/configitems.py
>>>>> @@ -13,6 +13,11 @@ from . import (
>>>>>        error,
>>>>>    )
>>>>>    
>>>>> +def loadconfigtable(ui, extname, configtable):
>>>>> +    """update config item known to the ui with the extension ones"""
>>>>> +    for section, items in  configtable.items():
>>>>> +        ui._knownconfig.setdefault(section, {}).update(items)
>>>>
>>>> So this effectively updates the global configitems.coreitems, and that's
>>>> unavoidable since the passed ui is a temporary object.
>>>
>>> Hum, yes good catch, we are missing some copying around here. However
>>> you mention the ui being a temporary object can you elaborate a bit?
>>> (I've some notion of the weird life cycle of UI object but I'm not sure
>>> I follow you here.
>>
>> extensions.loadall() is called with "lui", which can be a temporary copy of
>> the main "ui". "lui" exists for preloading repo hgrc in dispatch layer.
> 
> Anyway, I think loadconfigtable() needs to update some global table. Otherwise
> newly created ui object wouldn't see the defaults defined by loaded extensions

Do we need more than ui.copy() inheriting the configtable?

Cheers,
Yuya Nishihara - June 29, 2017, 3:17 p.m.
On Thu, 29 Jun 2017 16:37:44 +0200, Pierre-Yves David wrote:
> On 06/29/2017 04:29 PM, Yuya Nishihara wrote:
> > On Thu, 29 Jun 2017 23:11:57 +0900, Yuya Nishihara wrote:
> >> On Thu, 29 Jun 2017 15:59:17 +0200, Pierre-Yves David wrote:
> >>> On 06/29/2017 03:48 PM, Yuya Nishihara wrote:
> >>>> On Tue, 27 Jun 2017 15:00:31 +0200, Pierre-Yves David wrote:
> >>>>> # HG changeset patch
> >>>>> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> >>>>> # Date 1497700100 -7200
> >>>>> #      Sat Jun 17 13:48:20 2017 +0200
> >>>>> # Node ID 944f8e3191e6fa37f27d52d3c94d1ef702408ae4
> >>>>> # Parent  714ce79885e32305c45febf0da59b2ce2093967d
> >>>>> # 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 944f8e3191e6
> >>>>> configitems: add an official API for extensions to register config item
> >>>>>
> >>>>> Extensions can have a 'configtable' mapping and use
> >>>>> 'registrar.configitem(table)' to retrieve the registration function.
> >>>>>
> >>>>> This behave in the same way as the other way for extensions to register new
> >>>>> items (commands, colors, etc).
> >>>>>
> >>>>> diff --git a/mercurial/configitems.py b/mercurial/configitems.py
> >>>>> --- a/mercurial/configitems.py
> >>>>> +++ b/mercurial/configitems.py
> >>>>> @@ -13,6 +13,11 @@ from . import (
> >>>>>        error,
> >>>>>    )
> >>>>>    
> >>>>> +def loadconfigtable(ui, extname, configtable):
> >>>>> +    """update config item known to the ui with the extension ones"""
> >>>>> +    for section, items in  configtable.items():
> >>>>> +        ui._knownconfig.setdefault(section, {}).update(items)
> >>>>
> >>>> So this effectively updates the global configitems.coreitems, and that's
> >>>> unavoidable since the passed ui is a temporary object.
> >>>
> >>> Hum, yes good catch, we are missing some copying around here. However
> >>> you mention the ui being a temporary object can you elaborate a bit?
> >>> (I've some notion of the weird life cycle of UI object but I'm not sure
> >>> I follow you here.
> >>
> >> extensions.loadall() is called with "lui", which can be a temporary copy of
> >> the main "ui". "lui" exists for preloading repo hgrc in dispatch layer.
> > 
> > Anyway, I think loadconfigtable() needs to update some global table. Otherwise
> > newly created ui object wouldn't see the defaults defined by loaded extensions
> 
> Do we need more than ui.copy() inheriting the configtable?

Yes. hgweb may create a fresh new ui object, for example. It doesn't make much
sense to keep config tables locally because extensions are loaded per process.
Pierre-Yves David - June 29, 2017, 3:20 p.m.
On 06/29/2017 05:17 PM, Yuya Nishihara wrote:
> On Thu, 29 Jun 2017 16:37:44 +0200, Pierre-Yves David wrote:
>> On 06/29/2017 04:29 PM, Yuya Nishihara wrote:
>>> On Thu, 29 Jun 2017 23:11:57 +0900, Yuya Nishihara wrote:
>>>> On Thu, 29 Jun 2017 15:59:17 +0200, Pierre-Yves David wrote:
>>>>> On 06/29/2017 03:48 PM, Yuya Nishihara wrote:
>>>>>> On Tue, 27 Jun 2017 15:00:31 +0200, Pierre-Yves David wrote:
>>>>>>> # HG changeset patch
>>>>>>> # User Pierre-Yves David <pierre-yves.david@octobus.net>
>>>>>>> # Date 1497700100 -7200
>>>>>>> #      Sat Jun 17 13:48:20 2017 +0200
>>>>>>> # Node ID 944f8e3191e6fa37f27d52d3c94d1ef702408ae4
>>>>>>> # Parent  714ce79885e32305c45febf0da59b2ce2093967d
>>>>>>> # 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 944f8e3191e6
>>>>>>> configitems: add an official API for extensions to register config item
>>>>>>>
>>>>>>> Extensions can have a 'configtable' mapping and use
>>>>>>> 'registrar.configitem(table)' to retrieve the registration function.
>>>>>>>
>>>>>>> This behave in the same way as the other way for extensions to register new
>>>>>>> items (commands, colors, etc).
>>>>>>>
>>>>>>> diff --git a/mercurial/configitems.py b/mercurial/configitems.py
>>>>>>> --- a/mercurial/configitems.py
>>>>>>> +++ b/mercurial/configitems.py
>>>>>>> @@ -13,6 +13,11 @@ from . import (
>>>>>>>         error,
>>>>>>>     )
>>>>>>>     
>>>>>>> +def loadconfigtable(ui, extname, configtable):
>>>>>>> +    """update config item known to the ui with the extension ones"""
>>>>>>> +    for section, items in  configtable.items():
>>>>>>> +        ui._knownconfig.setdefault(section, {}).update(items)
>>>>>>
>>>>>> So this effectively updates the global configitems.coreitems, and that's
>>>>>> unavoidable since the passed ui is a temporary object.
>>>>>
>>>>> Hum, yes good catch, we are missing some copying around here. However
>>>>> you mention the ui being a temporary object can you elaborate a bit?
>>>>> (I've some notion of the weird life cycle of UI object but I'm not sure
>>>>> I follow you here.
>>>>
>>>> extensions.loadall() is called with "lui", which can be a temporary copy of
>>>> the main "ui". "lui" exists for preloading repo hgrc in dispatch layer.
>>>
>>> Anyway, I think loadconfigtable() needs to update some global table. Otherwise
>>> newly created ui object wouldn't see the defaults defined by loaded extensions
>>
>> Do we need more than ui.copy() inheriting the configtable?
> 
> Yes. hgweb may create a fresh new ui object, for example. It doesn't make much
> sense to keep config tables locally because extensions are loaded per process.

Okay I'll clarify all that in a follow up.

Thanks.

Patch

diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -13,6 +13,11 @@  from . import (
     error,
 )
 
+def loadconfigtable(ui, extname, configtable):
+    """update config item known to the ui with the extension ones"""
+    for section, items in  configtable.items():
+        ui._knownconfig.setdefault(section, {}).update(items)
+
 class configitem(object):
     """represent a known config item
 
diff --git a/mercurial/extensions.py b/mercurial/extensions.py
--- a/mercurial/extensions.py
+++ b/mercurial/extensions.py
@@ -18,6 +18,7 @@  from .i18n import (
 
 from . import (
     cmdutil,
+    configitems,
     encoding,
     error,
     pycompat,
@@ -263,6 +264,7 @@  def loadall(ui, whitelist=None):
     extraloaders = [
         ('cmdtable', commands, 'loadcmdtable'),
         ('colortable', color, 'loadcolortable'),
+        ('configtable', configitems, 'loadconfigtable'),
         ('filesetpredicate', fileset, 'loadpredicate'),
         ('revsetpredicate', revset, 'loadpredicate'),
         ('templatefilter', templatefilters, 'loadfilter'),
diff --git a/mercurial/registrar.py b/mercurial/registrar.py
--- a/mercurial/registrar.py
+++ b/mercurial/registrar.py
@@ -8,11 +8,19 @@ 
 from __future__ import absolute_import
 
 from . import (
+    configitems,
     error,
     pycompat,
     util,
 )
 
+# unlike the other registered items, config options are neither functions or
+# classes. Registering the option is just small function call.
+#
+# We still add the official API to the registrar module for consistency with
+# the other items extensions want might to register.
+configitem = configitems.getitemregister
+
 class _funcregistrarbase(object):
     """Base of decorator to register a function for specific purpose
 
diff --git a/tests/test-extension.t b/tests/test-extension.t
--- a/tests/test-extension.t
+++ b/tests/test-extension.t
@@ -5,6 +5,9 @@  Test basic extension support
   > from mercurial import commands, registrar
   > cmdtable = {}
   > command = registrar.command(cmdtable)
+  > configtable = {}
+  > configitem = registrar.configitem(configtable)
+  > configitem('tests', 'foo', default="Foo")
   > def uisetup(ui):
   >     ui.write("uisetup called\\n")
   >     ui.flush()
@@ -14,7 +17,9 @@  Test basic extension support
   >     ui.flush()
   > @command('foo', [], 'hg foo')
   > def foo(ui, *args, **kwargs):
-  >     ui.write("Foo\\n")
+  >     foo = ui.config('tests', 'foo')
+  >     ui.write(foo)
+  >     ui.write("\\n")
   > @command('bar', [], 'hg bar', norepo=True)
   > def bar(ui, *args, **kwargs):
   >     ui.write("Bar\\n")