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

[Boris FELD]

On 05/12/2018 06:08, Matt Harbison wrote:
> On Mon, 03 Dec 2018 03:37:07 -0500, Boris FELD
> <boris.feld at octobus.net> wrote:
>
>> On 12/2/18 12:37 PM, Yuya Nishihara wrote:
>>> On Fri, 30 Nov 2018 23:10:55 -0500, Matt Harbison wrote:
>>>> # HG changeset patch
>>>> # User Matt Harbison <matt_harbison at yahoo.com>
>>>> # Date 1543121473 18000
>>>> # Sat Nov 24 23:51:13 2018 -0500
>>>> # Node ID d5a04a8016a270dce028bddb2483509e0429f113
>>>> # Parent 33d30fb1e4ae52a283ef487ccf5dfbe59b8a5a68
>>>> extensions: import the exthelper class from evolve ff799015d62e
>>
>>>> It was copied unmodified, except:
>>>> - fix up the imports
>>>> - rename final_xxxsetup() -> finalxxxsetup() to appease checkcode
>>>> - avoid a [] default arg to wrapcommand()
>>>> - adjust the module name used to load the templates and revsets
>>
>> It would be nice if we kept the same API for the evolve and core, this
>> would help evolve to smoothly transition to the core one eventually.
>>
>> Should we keep the `final_` form in core or should we move to the more
>> compact one for evolve?
>
> I'd like to, but checkcode disagreed.  I know there's been talk about
> allowing '_' for readability; I'm not sure what the status of that
> is.  Not sure if anyone else wants to chime in about this.
>
>> The module name for templates and revsets could be a mandatory argument
>> to the exthelper object (at least the top level one).
>>
>> eh = exthelper('lfs')
>
> I thought about that, and I guess we can do that.  I was concerned
> that it's error prone, and I'm not sure how to make it mandatory for
> only the top level instance.

What about this:

- Extension name is either declared or None.
- a.merge(b) will complain if extension name is different (unless b's
one is None)
- the function that needs a name will crash if the extension name is
None (so, if we failed to declare it at the root level)
>
>> (note: We pushed a couple of the API improvement to evolve.)
>
> 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.
>
> Does the name seem reasonable with this wider audience?  I wanted to
> put it inside the extensions module, but it looks like there's a cycle
> around commands.py.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel