[PATCH 1 of 6] extensions: import the exthelper class from evolve ff799015d62e

[Boris FELD]

On 14/12/2018 11:54, Yuya Nishihara wrote:
> On Fri, 14 Dec 2018 10:18:42 +0000, Boris FELD wrote:
>> On 09/12/2018 05:29, Yuya Nishihara wrote:
>>> On Fri, 7 Dec 2018 15:44:44 +0100, Boris FELD wrote:
>>>>> I'm not sure what to do here.  Yuya objected to the fileset one in
>>>>> these.  I don't have a problem omitting this, the template and revset
>>>>> support, but I'm not sure what sort of problems that will cause in
>>>>> evolve.  I still think the command wrapping is worth keeping, due to
>>>>> the helper code to wrap other extension's commands.  Wrapping
>>>>> extension commands properly seems pretty rare (so few examples) and a
>>>>> bit obscure, so it seems better to not force the extension developer
>>>>> to recognize the problem and roll their own solution.
>>>>>
>>>>> I'd like to get agreement before sending off the next iteration.
>>>> Would it be possible to have the registrar command able to take an
>>>> exthelper object as parameter (as an alternative to their xxxtable).
>>>>
>>>> This way we don't need to duplicate the registrar's function API but we
>>>> keep the advantage of the exthelper.
>>> Can you elaborate?
>>>
>>> IIUC, the control flows of the registrar and the exthelper are different.
>>> With registrar, the extension loader loads functions from a static table,
>>> but with exthelper, functions are installed by a setup function. That makes
>>> me feel they are inherently incompatible.
>> The exthelper uses setup function because it predates the registrar. We
>> can change the underlying implementation. The core of the exthelper
>> class goals are simple:
>>
>> 1) provide a decorator based solution for all declarations
>> 2) allow to recursively combine instance from extension's submodules
>>
>> Registrar covers the decorator aspect (for everything with a registrar),
>> but not the recursive one.
>>
>> Here is what I have in mind, the exthelper object would be a valid
>> argument for registrar. The following code would be valid.
>>
>>   eh = exthelper('myextension')
>>   command = registar.command(eh)
>>   commandtable = eh.commandtable
>>
>>   @command(…)
> I don't think the registrar functions have to be overloaded for the exthelper.
> Instead, eh.commandtable can be passed in:
>
>   command = registrar.command(eh.commandtable)
>
> And if we want to make the eh.commandtable private, we can move the decorator
> to the exthelper:
>
>>>   class exthelper:
>>>       def __init__:
>>>           self.xxx = registrar.xxx(self._xxxtable)
>>>
>>>   # in top-level extension module
>>>   xxx = eh.xxx
>>>
>>>   @eh.xxx(...)
>>>   def ...
> This doesn't increase the complexity. In order to make registrar functions
> accept an exthelper instance, the exthelper would have to prepare tables
> for all the known registrar functions. So the exthelper has the knowledge
> about the registrar anyway. No need to make both registrar and exthelper
> know each other.

As long as we can still simply combine exthelper, I'm fine. Your
proposal works for me.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel