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

Matt Harbison mharbison72 at gmail.com
Fri Dec 21 21:17:39 EST 2018


On Fri, 14 Dec 2018 06:54:11 -0500, Yuya Nishihara <yuya at tcha.org> 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 ...

I think I'm confused.  It looks like commands and configitems are already  
registered this way.  The snippet of code that was quoted when you raised  
the initial question is configitem related.  So is the objection really  
just do the same thing for fileset/revset/template, and the rest is OK?


More information about the Mercurial-devel mailing list