[PATCH 1 of 7] dirstate: introduce new context manager for marking dirstate parent changes
Gregory Szorc
gregory.szorc at gmail.com
Thu May 18 20:37:25 EDT 2017
On Thu, May 18, 2017 at 3:19 PM, Augie Fackler <raf at durin42.com> wrote:
> # HG changeset patch
> # User Augie Fackler <augie at google.com>
> # Date 1495141830 14400
> # Thu May 18 17:10:30 2017 -0400
> # Node ID a69ba7abd7c7396205bbf44984fc5bd028a7f815
> # Parent 1f3972ed06833775ada5b23d9c90d1cbb240e4fd
> dirstate: introduce new context manager for marking dirstate parent changes
>
> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
> --- a/mercurial/dirstate.py
> +++ b/mercurial/dirstate.py
> @@ -8,6 +8,7 @@
> from __future__ import absolute_import
>
> import collections
> +import contextlib
> import errno
> import os
> import stat
> @@ -99,6 +100,18 @@ class dirstate(object):
> # for consistent view between _pl() and _read() invocations
> self._pendingmode = None
>
> + @contextlib.contextmanager
> + def parentchange(self):
> + '''Context manager for handling dirstate parents.
> +
> + If an exception occurs in the scope of the context manager,
> + the incoherent dirstate won't be written when wlock is
> + released.
> + '''
> + self._parentwriters += 1
> + yield
> + self._parentwriters -= 1
>
>
I assume the docstring explains why this isn't a try..finally, as one would
expect in most @contextmanager's. While the context is literally right
there today, I can't help but wonder if it warrants an inline comment
instead in case the docstring no longer shows up in the context of a future
diff. Or maybe it should be written to something like:
self._parentwriters += 1
try:
yield
except Exception:
# Don't decrement writer count because it shouldn't be written after
error.
raise
else:
self._parentwriters -= 1
(Yes, I'm likely being overly pedantic here. But not seeing try..finally in
@contextmanager really bothers my reviewer brain. IMO we should have a
style rule to enforce the pattern because it is such an easy mistake to
make.)
> def beginparentchange(self):
> '''Marks the beginning of a set of changes that involve changing
> the dirstate parents. If there is an exception during this time,
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170518/91a94ee8/attachment.html>
More information about the Mercurial-devel
mailing list