Patchwork formatter: add context manager interface for convenience

login
register
mail settings
Submitter Yuya Nishihara
Date Aug. 30, 2016, 3:05 p.m.
Message ID <41fea255bd1cda8d0f48.1472569558@mimosa>
Download mbox | patch
Permalink /patch/16479/
State Accepted
Headers show

Comments

Yuya Nishihara - Aug. 30, 2016, 3:05 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1472396405 -32400
#      Mon Aug 29 00:00:05 2016 +0900
# Node ID 41fea255bd1cda8d0f482b2582aec8a29f86c303
# Parent  90af59b40d8a007fb8811daf0c3e64aca43aa6b0
formatter: add context manager interface for convenience

And port "hg files" to test it.

As you can see, extra indent is necessary to port to this API. I don't think
we should switch every fm.formatter() call to "with" statement.
David Soria Parra - Aug. 30, 2016, 5:22 p.m.
On Wed, Aug 31, 2016 at 12:05:58AM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1472396405 -32400
> #      Mon Aug 29 00:00:05 2016 +0900
> # Node ID 41fea255bd1cda8d0f482b2582aec8a29f86c303
> # Parent  90af59b40d8a007fb8811daf0c3e64aca43aa6b0
> formatter: add context manager interface for convenience
> 
> And port "hg files" to test it.
> 
> As you can see, extra indent is necessary to port to this API. I don't think
> we should switch every fm.formatter() call to "with" statement.
> 

LGTM
Augie Fackler - Aug. 30, 2016, 9:23 p.m.
On Wed, Aug 31, 2016 at 12:05:58AM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1472396405 -32400
> #      Mon Aug 29 00:00:05 2016 +0900
> # Node ID 41fea255bd1cda8d0f482b2582aec8a29f86c303
> # Parent  90af59b40d8a007fb8811daf0c3e64aca43aa6b0
> formatter: add context manager interface for convenience

Nice, queued.

>
> And port "hg files" to test it.
>
> As you can see, extra indent is necessary to port to this API. I don't think
> we should switch every fm.formatter() call to "with" statement.

Agreed, but for simple cases it's a nice readability and correctness win.

>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -3963,15 +3963,11 @@ def files(ui, repo, *pats, **opts):
>      end = '\n'
>      if opts.get('print0'):
>          end = '\0'
> -    fm = ui.formatter('files', opts)
>      fmt = '%s' + end
>
>      m = scmutil.match(ctx, pats, opts)
> -    ret = cmdutil.files(ui, ctx, m, fm, fmt, opts.get('subrepos'))
> -
> -    fm.end()
> -
> -    return ret
> +    with ui.formatter('files', opts) as fm:
> +        return cmdutil.files(ui, ctx, m, fm, fmt, opts.get('subrepos'))
>
>  @command('^forget', walkopts, _('[OPTION]... FILE...'), inferrepo=True)
>  def forget(ui, repo, *pats, **opts):
> diff --git a/mercurial/formatter.py b/mercurial/formatter.py
> --- a/mercurial/formatter.py
> +++ b/mercurial/formatter.py
> @@ -52,6 +52,11 @@ class baseformatter(object):
>          self._item = None
>          # function to convert node to string suitable for this output
>          self.hexfunc = hex
> +    def __enter__(self):
> +        return self
> +    def __exit__(self, exctype, excvalue, traceback):
> +        if exctype is None:
> +            self.end()
>      def __nonzero__(self):
>          '''return False if we're not doing real templating so we can
>          skip extra work'''
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -3963,15 +3963,11 @@  def files(ui, repo, *pats, **opts):
     end = '\n'
     if opts.get('print0'):
         end = '\0'
-    fm = ui.formatter('files', opts)
     fmt = '%s' + end
 
     m = scmutil.match(ctx, pats, opts)
-    ret = cmdutil.files(ui, ctx, m, fm, fmt, opts.get('subrepos'))
-
-    fm.end()
-
-    return ret
+    with ui.formatter('files', opts) as fm:
+        return cmdutil.files(ui, ctx, m, fm, fmt, opts.get('subrepos'))
 
 @command('^forget', walkopts, _('[OPTION]... FILE...'), inferrepo=True)
 def forget(ui, repo, *pats, **opts):
diff --git a/mercurial/formatter.py b/mercurial/formatter.py
--- a/mercurial/formatter.py
+++ b/mercurial/formatter.py
@@ -52,6 +52,11 @@  class baseformatter(object):
         self._item = None
         # function to convert node to string suitable for this output
         self.hexfunc = hex
+    def __enter__(self):
+        return self
+    def __exit__(self, exctype, excvalue, traceback):
+        if exctype is None:
+            self.end()
     def __nonzero__(self):
         '''return False if we're not doing real templating so we can
         skip extra work'''