[PATCH 3 of 8] ignore: add ui to ignore stack

[Pierre-Yves David]

On 05/15/2015 02:26 PM, Matt Mackall wrote:
> On Wed, 2015-05-13 at 23:03 -0700, Pierre-Yves David wrote:
>>
>> On 05/13/2015 06:34 PM, Durham Goode wrote:
>>>
>>>
>>> On 5/13/15, 6:20 PM, "Pierre-Yves David" <pierre-yves.david at ens-lyon.org>
>>> wrote:
>>>
>>>>
>>>>
>>>> On 05/13/2015 08:13 AM, Durham Goode wrote:
>>>>> # HG changeset patch
>>>>> # User Durham Goode <durham at fb.com>
>>>>> # Date 1431468882 25200
>>>>> #      Tue May 12 15:14:42 2015 -0700
>>>>> # Node ID 5af3cb8572c462a3f21c45f7f8d9fdb0289dd373
>>>>> # Parent  1236d101b01d251715016df1b0ba53d3a37edfb8
>>>>> ignore: add ui to ignore stack
>>>>>
>>>>> In a future patch we will need access to the ui.config inside the
>>>>> ignore logic.
>>>>> This patch adds it to the functions.  A future patch will also remove
>>>>> the
>>>>> redundant 'warn' variable here.
>>>>
>>>> Do you have to pass the whole ui object there. Can't we just use the one
>>>> boolean of interested in there? I'm getting concerns at the `ui` object
>>>> getting its own cult, leaking everywhere, having small and simple piece
>>>> of code depending on it.
>>>
>>> I figured the fact that it needs to call warn() means the ui probably
>>> should've been passed in to begin with (since passing the warn function is
>>> funky).  In this case, since it needs both ui.warn and ui.config, it seems
>>> like passing the ui makes sense.
>>>
>>> If people disagree, I can change it.  I just think passing the ui is
>>> cleaner and the down sides are minimal.
>>
>> The warn function is a fairly agnostic way to "express error message if
>> any", we use this pattern in some other part of the code.
>>
>> Passing the ui object is definitely not a crazy approach. But I would
>> like to avoid the automatism of having ui object everywhere. I'll not
>> fight too much about that if the rest of the project disagree.
>
> It's not obvious to me that we need a config knob here, but perhaps I'm
> missing something.

I discussed with Durham about that today and here is the reasoning:

- We do not know how to combine regex to add the prefix.
- So we should not allow regex in included ignored by default.
- User who know about the limitation and might want regex anyway still 
want the feature.
- So we need a config knob to disable that by default but let power user 
enable it.

-- 
Pierre-Yves David