Fun with loading extensions

[Greg Ward]

On Tue, Jun 15, 2010 at 10:52 AM, Dirkjan Ochtman <dirkjan at ochtman.nl> wrote:
> Augie and I discussed this a little bit in IRC yesterday, and we
> seemed to agree that using wrapfunction() for (repo) methods is weird,
> and everyone should use subclassing and call super() instead.
> wrapfunction() is really intended for *functions*, not methods.

OK, but that only works if you want to wrap^H^H^H^Hoverride a method
of an object available in reposetup(): ui or repo.  In practice that's
fine, because people really only seem to use the "dynamic subclass"
trick on the repo object.

But if you want to wrap a method of, say, changectx, then you have no
choice but to use wrapfunction().  (I have one extension that does
this to insert info into the commit message: it's a bit tricky, but it
works.)

Here's a doc patch:

# HG changeset patch
# User Greg Ward <greg-hg at gerg.ca>
# Date 1276621462 14400
# Node ID e2bf80d7124a67507d988c84e5ff2b61673bdceb
# Parent  ad0a334eef163df857c36fdca2bd4daadba9df99
extensions: recommend against using wrapfunction() for repo methods.

Instead, all extensions should use the "dynamic subclass" trick:
subclass repo.__class__ and then replace repo.__class__ with your new
subclass.  This avoids conflicts that happen when one extension uses
wrapfunction and another uses subclassing to extend the same method of
localrepository.

diff --git a/mercurial/extensions.py b/mercurial/extensions.py
--- a/mercurial/extensions.py
+++ b/mercurial/extensions.py
@@ -124,6 +124,30 @@
     return entry

 def wrapfunction(container, funcname, wrapper):
+    '''Wrap the function named funcname in container by replacing it with
+    your wrapper.  container is typically a module, class, or instance.
+
+    wrapper will be called like
+      wrapper(orig, *args, **kwargs)
+    where orig is the original (wrapped) function, and *args, **kwargs are the
+    arguments passed to it.
+
+    Wrapping methods of the repository object is not recommended since it
+    conflicts with extensions that extend the repository by subclassing.
+    All extensions that need to extend methods of localrepository should
+    use this subclassing trick: namely, reposetup() should look like
+      def reposetup(ui, repo):
+          class myrepo(repo.__class__):
+              def whatever(self, *args, **kwargs):
+                  [...extension stuff...]
+                  super(myrepo, self).whatever(*args, **kwargs)
+                  [...extension stuff...]
+
+          repo.__class__ = myrepo
+
+    In general, combining wrapfunction() with subclassing does not work. Since
+    you cannot control what other extensions are loaded by your end users,
+    you should play nicely with others by using the subclass trick.'''
     def wrap(*args, **kwargs):
         return wrapper(origfn, *args, **kwargs)


If people like this, there's a wiki page or two that needs updating too.

Greg