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

durin42 (Augie Fackler) phabricator at mercurial-scm.org
Wed Feb 20 19:46:31 EST 2019


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


More information about the Mercurial-devel mailing list