[PATCH 1 of 2] extensions: add get contextmanager to avoid common pattern
Laurent Charignon
lcharignon at fb.com
Thu Jan 14 16:49:01 CST 2016
On Jan 14, 2016, at 2:45 PM, Gregory Szorc <gregory.szorc at gmail.com<mailto:gregory.szorc at gmail.com>> wrote:
On Thu, Jan 14, 2016 at 2:17 PM, Laurent Charignon <lcharignon at fb.com<mailto:lcharignon at fb.com>> wrote:
# HG changeset patch
# User Laurent Charignon <lcharignon at fb.com<mailto:lcharignon at fb.com>>
# Date 1452809804 28800
# Thu Jan 14 14:16:44 2016 -0800
# Node ID c1ba4521aea0906874fe7de5bee156d5070c0e56
# Parent e6e34c4e391668c5d8af8f98c004a27c77b1e2fa
extensions: add get contextmanager to avoid common pattern
Before this patch, people were sometimes using the following pattern:
try:
m = extensions.find(name)
...
except KeyError:
pass
This patch simplifies the pattern with context managers as follow:
with extensions.get(name) as m:
...
diff --git a/mercurial/extensions.py b/mercurial/extensions.py
--- a/mercurial/extensions.py
+++ b/mercurial/extensions.py
@@ -7,6 +7,7 @@
from __future__ import absolute_import
+from contextlib import contextmanager
import imp
import os
@@ -41,6 +42,13 @@ def extensions(ui=None):
if module and enabled(name):
yield name, module
+ at contextmanager
+def get(name):
+ try:
+ yield find(name)
+ except KeyError:
+ pass
+
Uh, I don't think this is valid. @contextmanager expects *something* to be returned:
import contextlib
@contextlib.contextmanager
def cm():
pass
with cm() as m:
print('hello')
$ python test.py
Traceback (most recent call last):
File "test.py", line 7, in <module>
with cm() as m:
File "/usr/local/Cellar/python/2.7.11/Frameworks/Python.framework/Versions/2.7/lib/python2.7/contextlib.py", line 17, in __enter__
return self.gen.next()
AttributeError: 'NoneType' object has no attribute 'next''
Oh, I missed that, we should add a test for it!
Even if it did work, it's not like the code inside the context manager in the next patch wouldn't get executed - it would just have None instead of the extension object.
Ok, I see.
Furthermore, context managers don't feel like the right fit here. Generic replacement for many try..finally, yes. For try..except, not really.
We have plenty of this pattern in our external extensions, that's why I did it in the first place.
Thanks,
Laurent
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20160114/97fe6b99/attachment-0001.html>
More information about the Mercurial-devel
mailing list