[PATCH 2 of 4] dispatch: add an additional extensibility point during dispatch

[Gregory Szorc]

On Wed, Feb 4, 2015 at 2:43 PM, Matt Mackall <mpm at selenic.com> wrote:

> On Wed, 2015-02-04 at 14:17 -0800, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc at gmail.com>
> > # Date 1423087102 28800
> > #      Wed Feb 04 13:58:22 2015 -0800
> > # Node ID f6866444b15b1bb1c5e24ea31660e1f8deb78a83
> > # Parent  c1b76e9330b1f39342899d8d53c3ddab388c3cb5
> > dispatch: add an additional extensibility point during dispatch
> >
> > An upcoming patch will introduce an ordering dependency between the
> > color and pager extensions. Specifically, one's uisetup function must
> > come before the other's.
> >
> > There is no easy way to accomplish guaranteed ordering of extension
> > functions. The Mercurial project has survived many years without an
> > explicit dependency system between extensions. Instead of inventing
> > one to solve this problem, we instead create a new, separate monkeypatch
> > point for the color extension to use to ensure its code runs after
> > pager's. It is ugly. But it is simple.
>
> This is pretty simple too:
>
> diff -r 7f375d2de945 mercurial/extensions.py
> --- a/mercurial/extensions.py   Mon Jan 26 14:30:12 2015 -0500
> +++ b/mercurial/extensions.py   Wed Feb 04 16:42:12 2015 -0600
> @@ -10,6 +10,7 @@
>  from i18n import _, gettext
>
>  _extensions = {}
> +_aftercallbacks = {}
>  _order = []
>  _ignore = ['hbisect', 'bookmarks', 'parentrevspec', 'interhg', 'inotify']
>
> @@ -87,6 +88,8 @@
>              mod = importh(name)
>      _extensions[shortname] = mod
>      _order.append(shortname)
> +    if shortname in _aftercallbacks:
> +        _aftercallbacks[shortname]()
>      return mod
>
>  def loadall(ui):
> @@ -123,6 +126,15 @@
>                      raise
>                  extsetup() # old extsetup with no ui argument
>
> +def aftersetup(extension, callback):
> +    """Run the specified function after the named extension is set up, or
> +    immediately"""
> +
> +    if extension in _extensions:
> +        callback()
> +    else:
> +        _aftercallbacks[extension] = callback
> +
>  def wrapcommand(table, command, wrapper):
>      '''Wrap the command named `command' in table
>
> Is it sufficient for your purposes? There are a few other places that
> want this sort of thing.


I think I can make this work. Although, the problem is still that pager and
color both do their magic at command dispatch time, not uisetup() time.

But there is one part that's a bit wonky. If color does an
aftersetup('pager') and pager is never loaded, the callback never fires. So
I guess we'd just double execute the color mode selection code in the
scenario where pager is loaded after color?

I started writing a patch that mucked about extensions._order. You could
even have extensions declare "after" or "before" dependencies via module
variables. I thought this was too much scope bloat.

Is there precedent for core code to call into bundled extensions like
color? It's very tempting to throw that into dispatch._runcommand...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150204/51fa86a4/attachment.html>