[Request] [+-- ] D10884: ui: add a context manager for silencing the ui (pushbuffer+popbuffer)

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Sat Jun 19 00:00:07 UTC 2021


martinvonz created this revision.
Herald added a reviewer: durin42.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  We often silence the ui by calling `ui.pushbuffer()` followed (a later
  in the code) by `ui.popbuffer()`. These places can be identified by
  the fact that they ignore the output returned from
  `ui.popbuffer()`. Let's create a context manager for these cases, to
  avoid repetition, and to avoid accidentally leaving the ui silent on
  exceptions. I deliberately called the new function `silent()` instead
  of `buffered()`, because it's just an implementation detail that it
  uses `pushbuffer()` and `popbuffer()`. We could later optimize it to
  not buffer the output.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D10884

AFFECTED FILES
  contrib/benchmarks/__init__.py
  hgext/histedit.py
  mercurial/commands.py
  mercurial/debugcommands.py
  mercurial/logcmdutil.py
  mercurial/repair.py
  mercurial/revset.py
  mercurial/ui.py
  tests/bruterebase.py

CHANGE DETAILS

diff --git a/tests/bruterebase.py b/tests/bruterebase.py
--- a/tests/bruterebase.py
+++ b/tests/bruterebase.py
@@ -48,26 +48,25 @@
             tr = repo.transaction(b'rebase')
             tr._report = lambda x: 0  # hide "transaction abort"
 
-            ui.pushbuffer()
-            try:
-                rebase.rebase(ui, repo, dest=dest, rev=[spec])
-            except error.Abort as ex:
-                summary = b'ABORT: %s' % ex.message
-            except Exception as ex:
-                summary = b'CRASH: %s' % ex
-            else:
-                # short summary about new nodes
-                cl = repo.changelog
-                descs = []
-                for rev in xrange(repolen, len(repo)):
-                    desc = b'%s:' % getdesc(rev)
-                    for prev in cl.parentrevs(rev):
-                        if prev > -1:
-                            desc += getdesc(prev)
-                    descs.append(desc)
-                descs.sort()
-                summary = b' '.join(descs)
-            ui.popbuffer()
+            with ui.silent():
+                try:
+                    rebase.rebase(ui, repo, dest=dest, rev=[spec])
+                except error.Abort as ex:
+                    summary = b'ABORT: %s' % ex.message
+                except Exception as ex:
+                    summary = b'CRASH: %s' % ex
+                else:
+                    # short summary about new nodes
+                    cl = repo.changelog
+                    descs = []
+                    for rev in xrange(repolen, len(repo)):
+                        desc = b'%s:' % getdesc(rev)
+                        for prev in cl.parentrevs(rev):
+                            if prev > -1:
+                                desc += getdesc(prev)
+                        descs.append(desc)
+                    descs.sort()
+                    summary = b' '.join(descs)
             repo.vfs.tryunlink(b'rebasestate')
 
             subsetdesc = b''.join(getdesc(rev) for rev in subset)
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -1146,6 +1146,14 @@
         self._fmsg = f
         self._fmsgout, self._fmsgerr = _selectmsgdests(self)
 
+    @contextlib.contextmanager
+    def silent(self, error=False, subproc=False, labeled=False):
+        self.pushbuffer(error=error, subproc=subproc, labeled=labeled)
+        try:
+            yield
+        finally:
+            self.popbuffer()
+
     def pushbuffer(self, error=False, subproc=False, labeled=False):
         """install a buffer to capture standard output of the ui object
 
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1872,9 +1872,10 @@
             revs = [repo.lookup(rev) for rev in revs]
         other = hg.peer(repo, {}, dest)
         try:
-            repo.ui.pushbuffer()
-            outgoing = discovery.findcommonoutgoing(repo, other, onlyheads=revs)
-            repo.ui.popbuffer()
+            with repo.ui.silent():
+                outgoing = discovery.findcommonoutgoing(
+                    repo, other, onlyheads=revs
+                )
         finally:
             other.close()
         missing.update(outgoing.missing)
diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -28,6 +28,7 @@
     pycompat,
     requirements,
     scmutil,
+    util,
 )
 from .utils import (
     hashutil,
@@ -239,19 +240,23 @@
                 ui.note(_(b"adding branch\n"))
                 f = vfs.open(tmpbundlefile, b"rb")
                 gen = exchange.readbundle(ui, f, tmpbundlefile, vfs)
-                if not repo.ui.verbose:
-                    # silence internal shuffling chatter
-                    repo.ui.pushbuffer()
-                tmpbundleurl = b'bundle:' + vfs.join(tmpbundlefile)
-                txnname = b'strip'
-                if not isinstance(gen, bundle2.unbundle20):
-                    txnname = b"strip\n%s" % urlutil.hidepassword(tmpbundleurl)
-                with repo.transaction(txnname) as tr:
-                    bundle2.applybundle(
-                        repo, gen, tr, source=b'strip', url=tmpbundleurl
-                    )
-                if not repo.ui.verbose:
-                    repo.ui.popbuffer()
+                # silence internal shuffling chatter
+                maybe_silent = (
+                    repo.ui.silent()
+                    if not repo.ui.verbose
+                    else util.nullcontextmanager()
+                )
+                with maybe_silent:
+                    tmpbundleurl = b'bundle:' + vfs.join(tmpbundlefile)
+                    txnname = b'strip'
+                    if not isinstance(gen, bundle2.unbundle20):
+                        txnname = b"strip\n%s" % urlutil.hidepassword(
+                            tmpbundleurl
+                        )
+                    with repo.transaction(txnname) as tr:
+                        bundle2.applybundle(
+                            repo, gen, tr, source=b'strip', url=tmpbundleurl
+                        )
                 f.close()
 
             with repo.transaction(b'repair') as tr:
diff --git a/mercurial/logcmdutil.py b/mercurial/logcmdutil.py
--- a/mercurial/logcmdutil.py
+++ b/mercurial/logcmdutil.py
@@ -93,9 +93,8 @@
             },
             b"merge-diff",
         ):
-            repo.ui.pushbuffer()
-            merge.merge(ctx.p2(), wc=wctx)
-            repo.ui.popbuffer()
+            with repo.ui.silent():
+                merge.merge(ctx.p2(), wc=wctx)
         return wctx
     else:
         return ctx.p1()
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -2754,9 +2754,9 @@
         changedelete = opts[b'changedelete']
         for path in ctx.walk(m):
             fctx = ctx[path]
-            try:
-                if not ui.debugflag:
-                    ui.pushbuffer(error=True)
+            with ui.silent(
+                error=True
+            ) if not ui.debugflag else util.nullcontextmanager():
                 tool, toolpath = filemerge._picktool(
                     repo,
                     ui,
@@ -2765,9 +2765,6 @@
                     b'l' in fctx.flags(),
                     changedelete,
                 )
-            finally:
-                if not ui.debugflag:
-                    ui.popbuffer()
             ui.write(b'%s = %s\n' % (path, tool))
 
 
@@ -4580,17 +4577,16 @@
             ui.write(_(b'creating http peer for wire protocol version 2\n'))
             # We go through makepeer() because we need an API descriptor for
             # the peer instance to be useful.
-            with ui.configoverride(
+            maybe_silent = (
+                ui.silent()
+                if opts[b'nologhandshake']
+                else util.nullcontextmanager()
+            )
+            with maybe_silent, ui.configoverride(
                 {(b'experimental', b'httppeer.advertise-v2'): True}
             ):
-                if opts[b'nologhandshake']:
-                    ui.pushbuffer()
-
                 peer = httppeer.makepeer(ui, path, opener=opener)
 
-                if opts[b'nologhandshake']:
-                    ui.popbuffer()
-
             if not isinstance(peer, httppeer.httpv2peer):
                 raise error.Abort(
                     _(
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -7241,9 +7241,8 @@
         if revs:
             revs = [other.lookup(rev) for rev in revs]
         ui.debug(b'comparing with %s\n' % urlutil.hidepassword(source))
-        repo.ui.pushbuffer()
-        commoninc = discovery.findcommonincoming(repo, other, heads=revs)
-        repo.ui.popbuffer()
+        with repo.ui.silent():
+            commoninc = discovery.findcommonincoming(repo, other, heads=revs)
         return source, sbranch, other, commoninc, commoninc[1]
 
     if needsincoming:
@@ -7287,11 +7286,10 @@
             common = commoninc
         if revs:
             revs = [repo.lookup(rev) for rev in revs]
-        repo.ui.pushbuffer()
-        outgoing = discovery.findcommonoutgoing(
-            repo, dother, onlyheads=revs, commoninc=common
-        )
-        repo.ui.popbuffer()
+        with repo.ui.silent():
+            outgoing = discovery.findcommonoutgoing(
+                repo, dother, onlyheads=revs, commoninc=common
+            )
         return dest, dbranch, dother, outgoing
 
     if needsoutgoing:
diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -575,9 +575,8 @@
         parentctx, but does not commit them."""
         repo = self.repo
         rulectx = repo[self.node]
-        repo.ui.pushbuffer()
-        hg.update(repo, self.state.parentctxnode, quietempty=True)
-        repo.ui.popbuffer()
+        with repo.ui.silent():
+            hg.update(repo, self.state.parentctxnode, quietempty=True)
         stats = applychanges(repo.ui, repo, rulectx, {})
         repo.dirstate.setbranch(rulectx.branch())
         if stats.unresolvedcount:
@@ -654,10 +653,9 @@
     if ctx.p1().node() == repo.dirstate.p1():
         # edits are "in place" we do not need to make any merge,
         # just applies changes on parent for editing
-        ui.pushbuffer()
-        cmdutil.revert(ui, repo, ctx, all=True)
-        stats = mergemod.updateresult(0, 0, 0, 0)
-        ui.popbuffer()
+        with ui.silent():
+            cmdutil.revert(ui, repo, ctx, all=True)
+            stats = mergemod.updateresult(0, 0, 0, 0)
     else:
         try:
             # ui.forcemerge is an internal variable, do not document
diff --git a/contrib/benchmarks/__init__.py b/contrib/benchmarks/__init__.py
--- a/contrib/benchmarks/__init__.py
+++ b/contrib/benchmarks/__init__.py
@@ -76,9 +76,8 @@
         ui, 'perfext', os.path.join(basedir, 'contrib', 'perf.py')
     )
     cmd = getattr(perfext, command)
-    ui.pushbuffer()
-    cmd(ui, repo, *args, **kwargs)
-    output = ui.popbuffer()
+    with ui.silent():
+        cmd(ui, repo, *args, **kwargs)
     match = outputre.search(output)
     if not match:
         raise ValueError("Invalid output {}".format(output))



To: martinvonz, durin42, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mercurial-scm.org/pipermail/mercurial-patches/attachments/20210619/459f7f0a/attachment-0001.html>


More information about the Mercurial-patches mailing list