[PATCH 1 of 4] ui: add a function to read configuration related to devel warning

Pierre-Yves David pierre-yves.david at ens-lyon.org
Wed Mar 11 18:34:06 CDT 2015



On 03/11/2015 03:39 PM, Matt Mackall wrote:
> On Wed, 2015-03-11 at 15:17 -0700, Pierre-Yves David wrote:
>>
>> On 03/11/2015 07:00 AM, Yuya Nishihara wrote:
>>> On Tue, 10 Mar 2015 21:56:47 -0700, Pierre-Yves David wrote:
>>>> # HG changeset patch
>>>> # User Pierre-Yves David <pierre-yves.david at fb.com>
>>>> # Date 1426047011 25200
>>>> #      Tue Mar 10 21:10:11 2015 -0700
>>>> # Node ID e9e42adc21a5c0a9bd68079bdd209b7a086ad046
>>>> # Parent  4ef4e3c3c00693868ba428e21ac092e559e4fcea
>>>> ui: add a function to read configuration related to devel warning
>>>>
>>>> We are about to introduce a class of configuration that will enable various
>>>> warnings targeted to developers. As we want both a wide and fine way to control
>>>> them, we introduce a special method to know if a warning of a given category
>>>> should be displayed or not.
>>>>
>>>> First user will be introduced in the next changeset.
>>>>
>>>> diff --git a/mercurial/ui.py b/mercurial/ui.py
>>>> --- a/mercurial/ui.py
>>>> +++ b/mercurial/ui.py
>>>> @@ -921,5 +921,11 @@ class ui(object):
>>>>
>>>>            ui.write(s, 'label') is equivalent to
>>>>            ui.write(ui.label(s, 'label')).
>>>>            '''
>>>>            return msg
>>>> +
>>>> +    def develflag(self, flag):
>>>> +        """check if a devel warning is requested for flag X
>>>> +
>>>> +        We do not just "config" because we want to support a 'all' value"""
>>>> +        return self.config('devel', 'all') or self.config('devel', flag)
>>>
>>> should be configbool() ?
>>
>> We are doing some magic with the 'all' value too. so this cannot just be
>> configbool. we can build a generic function to handle "all" value if
>> this becomes a general pattern.
>
> Still not clear. Why is it not:
>
>    return self.configbool('devel', 'all') or self.configbool('devel',
> flag)

1) Because if this is to be used in 24 different place, the odd for 
someone to forget the or statement is high.

But one can probably make a point that complication should probably be 
made when they are obviously a win instead of in advance.

> I'm unable to spot any 'all' magic.

2) I'm also expecting this function to grow some "all warnings but this 
blacklist" feature, but again, this probably fall into the "premature 
over-engineering bucket"

I'll drop this patch in the V2.

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list