# HG changeset patch # User Matt Harbison # Date 2019-01-17 05:16:00 # Node ID c9e1104e627229735796cfaa9dcbfc0e30082e75 # Parent 41f14e8f335fcc06d2ea0fe4160baabcb01b7cc1 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 diff --git a/hgext/lfs/__init__.py b/hgext/lfs/__init__.py --- a/hgext/lfs/__init__.py +++ b/hgext/lfs/__init__.py @@ -130,6 +130,7 @@ from mercurial.i18n import _ from mercurial import ( config, + context, error, exchange, extensions, @@ -329,6 +330,8 @@ def _resolverevlogstorevfsoptions(orig, 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 diff --git a/hgext/lfs/wrapper.py b/hgext/lfs/wrapper.py --- a/hgext/lfs/wrapper.py +++ b/hgext/lfs/wrapper.py @@ -208,7 +208,6 @@ def filectxisbinary(orig, self): 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()) diff --git a/mercurial/exthelper.py b/mercurial/exthelper.py --- a/mercurial/exthelper.py +++ b/mercurial/exthelper.py @@ -82,7 +82,6 @@ class exthelper(object): self._commandwrappers = [] self._extcommandwrappers = [] self._functionwrappers = [] - self._duckpunchers = [] self.cmdtable = {} self.command = registrar.command(self.cmdtable) self.configtable = {} @@ -102,7 +101,6 @@ class exthelper(object): 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 @@ class exthelper(object): - 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 @@ class exthelper(object): 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