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

Boris FELD boris.feld at octobus.net
Fri Dec 14 05:18:42 EST 2018


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(…)

This requires the registrar to recognize that its argument is an
exthelper instance and look for the right table there.

Such API could be easily improved by having the extensions logic
directly look for table inside the exthelper. Let us rename it to
"register" as it would better fit its new usage

  from mercurial import extension, registrar
  from . import subitem

  register = extensions.register('myextension')
  register.merge(subitem.register)

  command = registrar.command(register)

  @command(…)

>
> An alternative idea is to provide a thin registrar wrapper backed by an
> exthelper table.
>
>   class exthelper:
>       def __init__:
>           self.xxx = registrar.xxx(self._xxxtable)
>
>   # in top-level extension module
>   xxx = eh.xxx
>
>   @eh.xxx(...)
>   def ...
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel



More information about the Mercurial-devel mailing list