changeset 41279:c9e1104e6272

exthelper: drop the addattr() decorator Yuya pointed out that this goes against the typical advice to not add attributes to classes[1]. The evolve extension still uses this a handful of times, so maybe it should be brought back in the future if a general use is found. But it isn't nice to have a new helper API that can lead to easy problems. [1] https://www.mercurial-scm.org/pipermail/mercurial-devel/2018-December/126330.html
author Matt Harbison <matt_harbison@yahoo.com>
date Thu, 17 Jan 2019 00:16:00 -0500
parents 41f14e8f335f
children f4277a35c42c
files hgext/lfs/__init__.py hgext/lfs/wrapper.py mercurial/exthelper.py
diffstat 3 files changed, 3 insertions(+), 31 deletions(-) [+]
line wrap: on
line diff
--- a/hgext/lfs/__init__.py	Mon Jan 14 18:19:22 2019 +0100
+++ b/hgext/lfs/__init__.py	Thu Jan 17 00:16:00 2019 -0500
@@ -130,6 +130,7 @@
 
 from mercurial import (
     config,
+    context,
     error,
     exchange,
     extensions,
@@ -329,6 +330,8 @@
 def _extsetup(ui):
     wrapfilelog(filelog.filelog)
 
+    context.basefilectx.islfs = wrapper.filectxislfs
+
     scmutil.fileprefetchhooks.add('lfs', wrapper._prefetchfiles)
 
     # Make bundle choose changegroup3 instead of changegroup2. This affects
--- a/hgext/lfs/wrapper.py	Mon Jan 14 18:19:22 2019 +0100
+++ b/hgext/lfs/wrapper.py	Thu Jan 17 00:16:00 2019 -0500
@@ -208,7 +208,6 @@
         return bool(int(metadata.get('x-is-binary', 1)))
     return orig(self)
 
-@eh.addattr(context.basefilectx, 'islfs')
 def filectxislfs(self):
     return _islfs(self.filelog(), self.filenode())
 
--- a/mercurial/exthelper.py	Mon Jan 14 18:19:22 2019 +0100
+++ b/mercurial/exthelper.py	Thu Jan 17 00:16:00 2019 -0500
@@ -82,7 +82,6 @@
         self._commandwrappers = []
         self._extcommandwrappers = []
         self._functionwrappers = []
-        self._duckpunchers = []
         self.cmdtable = {}
         self.command = registrar.command(self.cmdtable)
         self.configtable = {}
@@ -102,7 +101,6 @@
         self._commandwrappers.extend(other._commandwrappers)
         self._extcommandwrappers.extend(other._extcommandwrappers)
         self._functionwrappers.extend(other._functionwrappers)
-        self._duckpunchers.extend(other._duckpunchers)
         self.cmdtable.update(other.cmdtable)
         for section, items in other.configtable.iteritems():
             if section in self.configtable:
@@ -129,8 +127,6 @@
         - Setup of pre-* and post-* hooks
         - pushkey setup
         """
-        for cont, funcname, func in self._duckpunchers:
-            setattr(cont, funcname, func)
         for command, wrapper, opts in self._commandwrappers:
             entry = extensions.wrapcommand(commands.table, command, wrapper)
             if opts:
@@ -302,29 +298,3 @@
             self._functionwrappers.append((container, funcname, wrapper))
             return wrapper
         return dec
-
-    def addattr(self, container, funcname):
-        """Decorated function is to be added to the container
-
-        This function takes two arguments, the container and the name of the
-        function to wrap. The wrapping is performed during `uisetup`.
-
-        Adding attributes to a container like this is discouraged, because the
-        container modification is visible even in repositories that do not
-        have the extension loaded.  Therefore, care must be taken that the
-        function doesn't make assumptions that the extension was loaded for the
-        current repository.  For `ui` and `repo` instances, a better option is
-        to subclass the instance in `uipopulate` and `reposetup` respectively.
-
-        https://www.mercurial-scm.org/wiki/WritingExtensions
-
-        example::
-
-            @eh.addattr(context.changectx, 'babar')
-            def babar(ctx):
-                return 'babar' in ctx.description
-        """
-        def dec(func):
-            self._duckpunchers.append((container, funcname, func))
-            return func
-        return dec