| Submitter | Yuya Nishihara |
|---|---|
| Date | Oct. 31, 2017, 2:32 p.m. |
| Message ID | <4f00958f5fc9d7ae9046.1509460349@mimosa> |
| Download | mbox | patch |
| Permalink | /patch/25315/ |
| State | Accepted |
| Headers | show |
Comments
> On Oct 31, 2017, at 10:32, Yuya Nishihara <yuya@tcha.org> wrote: > > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1509457050 -32400 > # Tue Oct 31 22:37:30 2017 +0900 > # Branch stable > # Node ID 4f00958f5fc9d7ae90468f6159dff7720d0f2b27 > # Parent 5caae380ff90735b1a1841a4f26147bf0f01351b > configitems: relax warning about unwanted default value > > The original condition was a bit harsh for extension authors since third-party > extensions need to preserve compatibility with older Mercurial versions, where > no defaults would be loaded from the configtable. So let's silence the warning > if the given default value matches, which should be harmless. > > diff --git a/mercurial/ui.py b/mercurial/ui.py > --- a/mercurial/ui.py > +++ b/mercurial/ui.py > @@ -471,12 +471,16 @@ class ui(object): > return value > > def _config(self, section, name, default=_unset, untrusted=False): > - value = default > + value = itemdefault = default > item = self._knownconfig.get(section, {}).get(name) > alternates = [(section, name)] > > if item is not None: > alternates.extend(item.alias) > + if callable(item.default): > + itemdefault = item.default() > + else: > + itemdefault = item.default > else: > msg = ("accessing unregistered config item: '%s.%s'") > msg %= (section, name) > @@ -490,12 +494,11 @@ class ui(object): > msg = "config item requires an explicit default value: '%s.%s'" > msg %= (section, name) > self.develwarn(msg, 2, 'warn-config-default') > - elif callable(item.default): > - value = item.default() > else: > - value = item.default > + value = itemdefault > elif (item is not None > - and item.default is not configitems.dynamicdefault): > + and item.default is not configitems.dynamicdefault > + and default != itemdefault): > msg = ("specifying a default value for a registered " > "config item: '%s.%s' '%s'") nit: should I edit the message to say "mismatched default" since that's what we're really complaining about? > msg %= (section, name, default) > diff --git a/tests/test-devel-warnings.t b/tests/test-devel-warnings.t > --- a/tests/test-devel-warnings.t > +++ b/tests/test-devel-warnings.t > @@ -352,17 +352,21 @@ Test warning on config option access and >> >> configitem('test', 'some', default='foo') >> configitem('test', 'dynamic', default=configitems.dynamicdefault) > + > configitem('test', 'callable', default=list) >> # overwrite a core config >> configitem('ui', 'quiet', default=False) >> configitem('ui', 'interactive', default=None) >> >> @command(b'buggyconfig') >> def cmdbuggyconfig(ui, repo): > - > repo.ui.config('ui', 'quiet', False) > - > repo.ui.config('ui', 'interactive', None) > + > repo.ui.config('ui', 'quiet', True) > + > repo.ui.config('ui', 'interactive', False) > + > repo.ui.config('test', 'some', 'bar') >> repo.ui.config('test', 'some', 'foo') >> repo.ui.config('test', 'dynamic', 'some-required-default') >> repo.ui.config('test', 'dynamic') > + > repo.ui.config('test', 'callable', []) > + > repo.ui.config('test', 'callable', 'foo') >> repo.ui.config('test', 'unregistered') >> repo.ui.config('unregistered', 'unregistered') >> EOF > @@ -370,10 +374,11 @@ Test warning on config option access and > $ hg --config "extensions.buggyconfig=${TESTTMP}/buggyconfig.py" buggyconfig > devel-warn: extension 'buggyconfig' overwrite config item 'ui.interactive' at: */mercurial/extensions.py:* (_loadextra) (glob) > devel-warn: extension 'buggyconfig' overwrite config item 'ui.quiet' at: */mercurial/extensions.py:* (_loadextra) (glob) > - devel-warn: specifying a default value for a registered config item: 'ui.quiet' 'False' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) > - devel-warn: specifying a default value for a registered config item: 'ui.interactive' 'None' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) > - devel-warn: specifying a default value for a registered config item: 'test.some' 'foo' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) > + devel-warn: specifying a default value for a registered config item: 'ui.quiet' 'True' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) > + devel-warn: specifying a default value for a registered config item: 'ui.interactive' 'False' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) > + devel-warn: specifying a default value for a registered config item: 'test.some' 'bar' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) > devel-warn: config item requires an explicit default value: 'test.dynamic' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) > + devel-warn: specifying a default value for a registered config item: 'test.callable' 'foo' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) > devel-warn: accessing unregistered config item: 'test.unregistered' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) > devel-warn: accessing unregistered config item: 'unregistered.unregistered' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On Tue, 31 Oct 2017 10:44:42 -0400, Augie Fackler wrote: > > On Oct 31, 2017, at 10:32, Yuya Nishihara <yuya@tcha.org> wrote: > > # HG changeset patch > > # User Yuya Nishihara <yuya@tcha.org> > > # Date 1509457050 -32400 > > # Tue Oct 31 22:37:30 2017 +0900 > > # Branch stable > > # Node ID 4f00958f5fc9d7ae90468f6159dff7720d0f2b27 > > # Parent 5caae380ff90735b1a1841a4f26147bf0f01351b > > configitems: relax warning about unwanted default value > > > > The original condition was a bit harsh for extension authors since third-party > > extensions need to preserve compatibility with older Mercurial versions, where > > no defaults would be loaded from the configtable. So let's silence the warning > > if the given default value matches, which should be harmless. > > > > diff --git a/mercurial/ui.py b/mercurial/ui.py > > --- a/mercurial/ui.py > > +++ b/mercurial/ui.py > > @@ -471,12 +471,16 @@ class ui(object): > > return value > > > > def _config(self, section, name, default=_unset, untrusted=False): > > - value = default > > + value = itemdefault = default > > item = self._knownconfig.get(section, {}).get(name) > > alternates = [(section, name)] > > > > if item is not None: > > alternates.extend(item.alias) > > + if callable(item.default): > > + itemdefault = item.default() > > + else: > > + itemdefault = item.default > > else: > > msg = ("accessing unregistered config item: '%s.%s'") > > msg %= (section, name) > > @@ -490,12 +494,11 @@ class ui(object): > > msg = "config item requires an explicit default value: '%s.%s'" > > msg %= (section, name) > > self.develwarn(msg, 2, 'warn-config-default') > > - elif callable(item.default): > > - value = item.default() > > else: > > - value = item.default > > + value = itemdefault > > elif (item is not None > > - and item.default is not configitems.dynamicdefault): > > + and item.default is not configitems.dynamicdefault > > + and default != itemdefault): > > msg = ("specifying a default value for a registered " > > "config item: '%s.%s' '%s'") > > nit: should I edit the message to say "mismatched default" since that's what we're really complaining about? I don't care much about that, but "specifying a mismatched default" sounds more correct. Can you update it in flight?
> On Oct 31, 2017, at 11:30, Yuya Nishihara <yuya@tcha.org> wrote: > > On Tue, 31 Oct 2017 10:44:42 -0400, Augie Fackler wrote: >>> On Oct 31, 2017, at 10:32, Yuya Nishihara <yuya@tcha.org> wrote: >>> # HG changeset patch >>> # User Yuya Nishihara <yuya@tcha.org> >>> # Date 1509457050 -32400 >>> # Tue Oct 31 22:37:30 2017 +0900 >>> # Branch stable >>> # Node ID 4f00958f5fc9d7ae90468f6159dff7720d0f2b27 >>> # Parent 5caae380ff90735b1a1841a4f26147bf0f01351b >>> configitems: relax warning about unwanted default value >>> >>> The original condition was a bit harsh for extension authors since third-party >>> extensions need to preserve compatibility with older Mercurial versions, where >>> no defaults would be loaded from the configtable. So let's silence the warning >>> if the given default value matches, which should be harmless. >>> >>> diff --git a/mercurial/ui.py b/mercurial/ui.py >>> --- a/mercurial/ui.py >>> +++ b/mercurial/ui.py >>> @@ -471,12 +471,16 @@ class ui(object): >>> return value >>> >>> def _config(self, section, name, default=_unset, untrusted=False): >>> - value = default >>> + value = itemdefault = default >>> item = self._knownconfig.get(section, {}).get(name) >>> alternates = [(section, name)] >>> >>> if item is not None: >>> alternates.extend(item.alias) >>> + if callable(item.default): >>> + itemdefault = item.default() >>> + else: >>> + itemdefault = item.default >>> else: >>> msg = ("accessing unregistered config item: '%s.%s'") >>> msg %= (section, name) >>> @@ -490,12 +494,11 @@ class ui(object): >>> msg = "config item requires an explicit default value: '%s.%s'" >>> msg %= (section, name) >>> self.develwarn(msg, 2, 'warn-config-default') >>> - elif callable(item.default): >>> - value = item.default() >>> else: >>> - value = item.default >>> + value = itemdefault >>> elif (item is not None >>> - and item.default is not configitems.dynamicdefault): >>> + and item.default is not configitems.dynamicdefault >>> + and default != itemdefault): >>> msg = ("specifying a default value for a registered " >>> "config item: '%s.%s' '%s'") >> >> nit: should I edit the message to say "mismatched default" since that's what we're really complaining about? > > I don't care much about that, but "specifying a mismatched default" sounds > more correct. Can you update it in flight? Queued with that fixed in flight, many thanks.
Patch
diff --git a/mercurial/ui.py b/mercurial/ui.py --- a/mercurial/ui.py +++ b/mercurial/ui.py @@ -471,12 +471,16 @@ class ui(object): return value def _config(self, section, name, default=_unset, untrusted=False): - value = default + value = itemdefault = default item = self._knownconfig.get(section, {}).get(name) alternates = [(section, name)] if item is not None: alternates.extend(item.alias) + if callable(item.default): + itemdefault = item.default() + else: + itemdefault = item.default else: msg = ("accessing unregistered config item: '%s.%s'") msg %= (section, name) @@ -490,12 +494,11 @@ class ui(object): msg = "config item requires an explicit default value: '%s.%s'" msg %= (section, name) self.develwarn(msg, 2, 'warn-config-default') - elif callable(item.default): - value = item.default() else: - value = item.default + value = itemdefault elif (item is not None - and item.default is not configitems.dynamicdefault): + and item.default is not configitems.dynamicdefault + and default != itemdefault): msg = ("specifying a default value for a registered " "config item: '%s.%s' '%s'") msg %= (section, name, default) diff --git a/tests/test-devel-warnings.t b/tests/test-devel-warnings.t --- a/tests/test-devel-warnings.t +++ b/tests/test-devel-warnings.t @@ -352,17 +352,21 @@ Test warning on config option access and > > configitem('test', 'some', default='foo') > configitem('test', 'dynamic', default=configitems.dynamicdefault) + > configitem('test', 'callable', default=list) > # overwrite a core config > configitem('ui', 'quiet', default=False) > configitem('ui', 'interactive', default=None) > > @command(b'buggyconfig') > def cmdbuggyconfig(ui, repo): - > repo.ui.config('ui', 'quiet', False) - > repo.ui.config('ui', 'interactive', None) + > repo.ui.config('ui', 'quiet', True) + > repo.ui.config('ui', 'interactive', False) + > repo.ui.config('test', 'some', 'bar') > repo.ui.config('test', 'some', 'foo') > repo.ui.config('test', 'dynamic', 'some-required-default') > repo.ui.config('test', 'dynamic') + > repo.ui.config('test', 'callable', []) + > repo.ui.config('test', 'callable', 'foo') > repo.ui.config('test', 'unregistered') > repo.ui.config('unregistered', 'unregistered') > EOF @@ -370,10 +374,11 @@ Test warning on config option access and $ hg --config "extensions.buggyconfig=${TESTTMP}/buggyconfig.py" buggyconfig devel-warn: extension 'buggyconfig' overwrite config item 'ui.interactive' at: */mercurial/extensions.py:* (_loadextra) (glob) devel-warn: extension 'buggyconfig' overwrite config item 'ui.quiet' at: */mercurial/extensions.py:* (_loadextra) (glob) - devel-warn: specifying a default value for a registered config item: 'ui.quiet' 'False' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) - devel-warn: specifying a default value for a registered config item: 'ui.interactive' 'None' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) - devel-warn: specifying a default value for a registered config item: 'test.some' 'foo' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) + devel-warn: specifying a default value for a registered config item: 'ui.quiet' 'True' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) + devel-warn: specifying a default value for a registered config item: 'ui.interactive' 'False' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) + devel-warn: specifying a default value for a registered config item: 'test.some' 'bar' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) devel-warn: config item requires an explicit default value: 'test.dynamic' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) + devel-warn: specifying a default value for a registered config item: 'test.callable' 'foo' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) devel-warn: accessing unregistered config item: 'test.unregistered' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob) devel-warn: accessing unregistered config item: 'unregistered.unregistered' at: $TESTTMP/buggyconfig.py:* (cmdbuggyconfig) (glob)