[PATCH 1 of 6 evolve-ext-V2] evolve: mechanism to load some commands selectively

[Pierre-Yves David]

On 06/05/2015 04:28 PM, Laurent Charignon wrote:
>
>> On Jun 4, 2015, at 10:32 PM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
>>
>>
>>
>> On 06/04/2015 04:09 PM, Laurent Charignon wrote:
>>> # HG changeset patch
>>> # User Laurent Charignon <lcharignon at fb.com>
>>> # Date 1432164945 25200
>>> #      Wed May 20 16:35:45 2015 -0700
>>> # Node ID a72a896bfc902806d3847915f29c2ffe1db8c37b
>>> # Parent  82dd98428b8d85ea0c9acefebcff40ea59902c66
>>> evolve: mechanism to load some commands selectively

So to clarigy, I would like the last 3 changesets of this series 
(respect allow unstable) before the first 3 (because the last three are 
already ready)

>>> +    evolvecommands = ui.configlist('experimental', 'evolutioncommands')
>>
>> I would probably set a default value of 'all' here (yes this is a change compared to the 'commands' entry in evolution.
>
> - I don't understand what you mean with the sentence in the parentheses.
> - At this point (1/6), all has no meaning. Just to make sure, you suggest to make 'all' the default value in the patch that introduces 'all' as a value?

I'm suggesting that if no value is supplied (≠ '') all commands should 
be enabled.

What do you think?

>>>       evolveopts = ui.configlist('experimental', 'evolution')
>>> -    if evolveopts and (commandopt not in evolveopts and
>>> -                       'all' not in evolveopts):
>>> -        cmdtable.clear()
>>> +    if not evolveopts or 'all' in evolveopts:
>>> +        # Evolve is fully loaded, don't disable commands
>>> +        return
>>> +    else:
>>> +        whitelist = set()
>>> +        for cmd in evolvecommands:
>>> +            matchingevolvecommands = [e for e in cmdtable.keys() if cmd in e]
>>> +            if not matchingevolvecommands:
>>> +                raise error.Abort(_('Unknown command'))
>>
>> - do not capitalize error message
>
> Ok
>
>> - If we do not specify the command who is unkown, the user will flip his laptop.
>
> Good suggestion
>
>
>>
>>> +            elif len(matchingevolvecommands) > 1:
>>> +                raise error.Abort(_('Ambiguous command specification'))
>>
>> ditto
>
> Ok
>
>>
>>> +            else:
>>> +                whitelist.add(matchingevolvecommands[0])
>>> +        for disabledcmd in set(cmdtable) - whitelist:
>>
>> You could also start with disabledcmd and remove element of the set as they match.
>
> It should come down to the same thing, as you suggest my suggestion is a little less safe (blacklisting vs whitelisting), I might revisit it in V3.

You are building a set of match, and subtracting to the list of all 
known commands. Why not remove then on the go?

-- 
Pierre-Yves David