D5994: cleanup: prefer nested context managers to \-continuations

[durin42 (Augie Fackler)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHG1eb2fc21da12: cleanup: prefer nested context managers to 
\-continuations (authored by durin42, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5994?vs=14164=14179

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

AFFECTED FILES
  hgext/largefiles/overrides.py
  hgext/narrow/narrowcommands.py
  mercurial/exchange.py

CHANGE DETAILS

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -556,18 +556,18 @@
% stringutil.forcebytestr(err))
 pushop.ui.debug(msg)
 
-with wlock or util.nullcontextmanager(), \
-lock or util.nullcontextmanager(), \
-pushop.trmanager or util.nullcontextmanager():
-pushop.repo.checkpush(pushop)
-_checkpublish(pushop)
-_pushdiscovery(pushop)
-if not _forcebundle1(pushop):
-_pushbundle2(pushop)
-_pushchangeset(pushop)
-_pushsyncphase(pushop)
-_pushobsolete(pushop)
-_pushbookmark(pushop)
+with wlock or util.nullcontextmanager():
+with lock or util.nullcontextmanager():
+with pushop.trmanager or util.nullcontextmanager():
+pushop.repo.checkpush(pushop)
+_checkpublish(pushop)
+_pushdiscovery(pushop)
+if not _forcebundle1(pushop):
+_pushbundle2(pushop)
+_pushchangeset(pushop)
+_pushsyncphase(pushop)
+_pushobsolete(pushop)
+_pushbookmark(pushop)
 
 if repo.ui.configbool('experimental', 'remotenames'):
 logexchange.pullremotenames(repo, remote)
diff --git a/hgext/narrow/narrowcommands.py b/hgext/narrow/narrowcommands.py
--- a/hgext/narrow/narrowcommands.py
+++ b/hgext/narrow/narrowcommands.py
@@ -278,9 +278,9 @@
 p1, p2 = ds.p1(), ds.p2()
 with ds.parentchange():
 ds.setparents(node.nullid, node.nullid)
-with wrappedextraprepare,\
- repo.ui.configoverride(overrides, 'widen'):
-exchange.pull(repo, remote, heads=common)
+with wrappedextraprepare:
+with repo.ui.configoverride(overrides, 'widen'):
+exchange.pull(repo, remote, heads=common)
 with ds.parentchange():
 ds.setparents(p1, p2)
 else:
@@ -296,11 +296,11 @@
 'ellipses': False,
 }).result()
 
-with repo.transaction('widening') as tr,\
- repo.ui.configoverride(overrides, 'widen'):
-tgetter = lambda: tr
-bundle2.processbundle(repo, bundle,
-transactiongetter=tgetter)
+with repo.transaction('widening') as tr:
+with repo.ui.configoverride(overrides, 'widen'):
+tgetter = lambda: tr
+bundle2.processbundle(repo, bundle,
+transactiongetter=tgetter)
 
 with repo.transaction('widening'):
 repo.setnewnarrowpats()
diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -664,9 +664,9 @@
   _('destination largefile already exists'))
 copiedfiles.append((src, dest))
 orig(src, dest, *args, **kwargs)
-with extensions.wrappedfunction(util, 'copyfile', overridecopyfile), \
- extensions.wrappedfunction(scmutil, 'match', overridematch):
-result += orig(ui, repo, listpats, opts, rename)
+with extensions.wrappedfunction(util, 'copyfile', overridecopyfile):
+with extensions.wrappedfunction(scmutil, 'match', overridematch):
+result += orig(ui, repo, listpats, opts, rename)
 
 lfdirstate = lfutil.openlfdirstate(ui, repo)
 for (src, dest) in copiedfiles:



To: durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D5994: cleanup: prefer nested context managers to \-continuations

[durin42 (Augie Fackler)]

durin42 added a comment.


  In https://phab.mercurial-scm.org/D5994#87506, @martinvonz wrote:
  
  > > I'd prefer Python accept a tuple of context managers
  >
  > I think I'd also prefer if our context managers didn't acquire the resource 
in `__init__`. That would let us write it with less indentation as:
  >
  >   wlock = wlock or util.nullcontextmanager()
  >   lock = lock or util.nullcontextmanager()
  >   trmanager = pushop.trmanager or util.nullcontextmanager()
  >   with wlock, lock, trmanager:
  >
  >
  > (In this case we probably could do that anyway since we don't seem to worry 
about the risk of exceptions from the time we take the lock until we enter the 
context manager.)
  >
  > I don't care much either way about the patch itself. I'll wait a little to 
see if someone else cares more, but otherwise I'll take.
  
  
  That sounds like a good cleanup anyway - probably taking the resource is a 
vestige of when we were on around Python 2.3 and mpm had this discipline of 
using `del` to free things like locks. It worked, but isn't idiomatic today 
(I'm honestly not sure if it was then - but mpm is where I learned it.)

REPOSITORY
  rHG Mercurial

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

To: durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D5994: cleanup: prefer nested context managers to \-continuations

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  > I'd prefer Python accept a tuple of context managers
  
  I think I'd also prefer if our context managers didn't acquire the resource 
in `__init__`. That would let us write it with less indentation as:
  
wlock = wlock or util.nullcontextmanager()
lock = lock or util.nullcontextmanager()
trmanager = pushop.trmanager or util.nullcontextmanager()
with wlock, lock, trmanager:
  
  (In this case we probably could do that anyway since we don't seem to worry 
about the risk of exceptions from the time we take the lock until we enter the 
context manager.)
  
  I don't care much either way about the patch itself. I'll wait a little to 
see if someone else cares more, but otherwise I'll take.

REPOSITORY
  rHG Mercurial

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

To: durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D5994: cleanup: prefer nested context managers to \-continuations

[durin42 (Augie Fackler)]

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

REVISION SUMMARY
  I'd prefer Python accept a tuple of context managers, but alas it
  isn't meant to be. This will have to suffice.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  hgext/largefiles/overrides.py
  hgext/narrow/narrowcommands.py
  mercurial/exchange.py

CHANGE DETAILS

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -556,18 +556,18 @@
% stringutil.forcebytestr(err))
 pushop.ui.debug(msg)
 
-with wlock or util.nullcontextmanager(), \
-lock or util.nullcontextmanager(), \
-pushop.trmanager or util.nullcontextmanager():
-pushop.repo.checkpush(pushop)
-_checkpublish(pushop)
-_pushdiscovery(pushop)
-if not _forcebundle1(pushop):
-_pushbundle2(pushop)
-_pushchangeset(pushop)
-_pushsyncphase(pushop)
-_pushobsolete(pushop)
-_pushbookmark(pushop)
+with wlock or util.nullcontextmanager():
+with lock or util.nullcontextmanager():
+with pushop.trmanager or util.nullcontextmanager():
+pushop.repo.checkpush(pushop)
+_checkpublish(pushop)
+_pushdiscovery(pushop)
+if not _forcebundle1(pushop):
+_pushbundle2(pushop)
+_pushchangeset(pushop)
+_pushsyncphase(pushop)
+_pushobsolete(pushop)
+_pushbookmark(pushop)
 
 if repo.ui.configbool('experimental', 'remotenames'):
 logexchange.pullremotenames(repo, remote)
diff --git a/hgext/narrow/narrowcommands.py b/hgext/narrow/narrowcommands.py
--- a/hgext/narrow/narrowcommands.py
+++ b/hgext/narrow/narrowcommands.py
@@ -278,9 +278,9 @@
 p1, p2 = ds.p1(), ds.p2()
 with ds.parentchange():
 ds.setparents(node.nullid, node.nullid)
-with wrappedextraprepare,\
- repo.ui.configoverride(overrides, 'widen'):
-exchange.pull(repo, remote, heads=common)
+with wrappedextraprepare:
+with repo.ui.configoverride(overrides, 'widen'):
+exchange.pull(repo, remote, heads=common)
 with ds.parentchange():
 ds.setparents(p1, p2)
 else:
@@ -296,11 +296,11 @@
 'ellipses': False,
 }).result()
 
-with repo.transaction('widening') as tr,\
- repo.ui.configoverride(overrides, 'widen'):
-tgetter = lambda: tr
-bundle2.processbundle(repo, bundle,
-transactiongetter=tgetter)
+with repo.transaction('widening') as tr:
+with repo.ui.configoverride(overrides, 'widen'):
+tgetter = lambda: tr
+bundle2.processbundle(repo, bundle,
+transactiongetter=tgetter)
 
 with repo.transaction('widening'):
 repo.setnewnarrowpats()
diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -664,9 +664,9 @@
   _('destination largefile already exists'))
 copiedfiles.append((src, dest))
 orig(src, dest, *args, **kwargs)
-with extensions.wrappedfunction(util, 'copyfile', overridecopyfile), \
- extensions.wrappedfunction(scmutil, 'match', overridematch):
-result += orig(ui, repo, listpats, opts, rename)
+with extensions.wrappedfunction(util, 'copyfile', overridecopyfile):
+with extensions.wrappedfunction(scmutil, 'match', overridematch):
+result += orig(ui, repo, listpats, opts, rename)
 
 lfdirstate = lfutil.openlfdirstate(ui, repo)
 for (src, dest) in copiedfiles:



To: durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel