[PATCH 1 of 2] extensions: add get contextmanager to avoid common pattern

[Gregory Szorc]

On Thu, Jan 14, 2016 at 2:17 PM, Laurent Charignon <lcharignon at fb.com>
wrote:

> # HG changeset patch
> # User Laurent Charignon <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'

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.

Furthermore, context managers don't feel like the right fit here. Generic
replacement for many try..finally, yes. For try..except, not really.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20160114/0aee12b0/attachment.html>