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

Yuya Nishihara yuya at tcha.org
Fri Dec 14 06:54:11 EST 2018


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.


More information about the Mercurial-devel mailing list