This is an archive of the discontinued Mercurial Phabricator instance.

Differential D66

histedit: extract InterventionRequired transaction handling to utils
ClosedPublic

Authored by martinvonz on Jul 12 2017, 5:38 PM.

Details

Reviewers
durham
Group Reviewers
hg-reviewers
Commits
rHGfad6852cf879: histedit: extract InterventionRequired transaction handling to utils
Summary

rebase will have similar logic, so let's extract it. Besides, it makes
the histedit code more readable.

We may want to parametrize acceptintervention() by the exception(s)
that should result in transaction close.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.Jul 12 2017, 5:38 PM
martinvonz updated this revision to Diff 100.Jul 12 2017, 7:58 PM
lothiraldan added a subscriber: lothiraldan.Jul 13 2017, 4:42 AM

LGTM apart from the condition change

mercurial/util.py
599

The initial check was slighty more precise (if tr is not None), any specific reason for the change?

martinvonz added inline comments.Jul 13 2017, 12:05 PM
mercurial/util.py
599

We generally prefer "not X" over "is not None" when they're functionally equivalent (and I do think they are here).

durham accepted this revision.Jul 13 2017, 3:49 PM
durham added inline comments.
mercurial/util.py
594

Technically this will apply to dsguard as well, so 'tr' isn't quite precise. But I think it's better to use 'tr' than invent some third term.

martinvonz updated this revision to Diff 116.Jul 13 2017, 4:01 PM
Closed by commit rHGfad6852cf879: histedit: extract InterventionRequired transaction handling to utils (authored by martinvonz). · Explain WhyJul 14 2017, 1:25 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/histedit.py (13 lines)
Mmercurial/util.py (19 lines)

Diff 94

hgext/histedit.py

# Don't use a 'with' for the transaction, since actions may close # Don't use a 'with' for the transaction, since actions may close
# and reopen a transaction. For example, if the action executes an # and reopen a transaction. For example, if the action executes an
# external process it may choose to commit the transaction first. # external process it may choose to commit the transaction first.
tr = repo.transaction('histedit') tr = repo.transaction('histedit')
# Force an initial state file write, so the user can run --abort/continue # Force an initial state file write, so the user can run --abort/continue
# even if there's an exception before the first transaction serialize. # even if there's an exception before the first transaction serialize.
state.write() state.write()
try: with util.acceptintervention(tr):
while state.actions: while state.actions:
state.write(tr=tr) state.write(tr=tr)
actobj = state.actions[0] actobj = state.actions[0]
pos += 1 pos += 1
ui.progress(_("editing"), pos, actobj.torule(), ui.progress(_("editing"), pos, actobj.torule(),
_('changes'), total) _('changes'), total)
ui.debug('histedit: processing %s %s\n' % (actobj.verb,\ ui.debug('histedit: processing %s %s\n' % (actobj.verb,\
actobj.torule())) actobj.torule()))
parentctx, replacement_ = actobj.run() parentctx, replacement_ = actobj.run()
state.parentctxnode = parentctx.node() state.parentctxnode = parentctx.node()
state.replacements.extend(replacement_) state.replacements.extend(replacement_)
state.actions.pop(0) state.actions.pop(0)
if tr is not None:
tr.close()
except error.InterventionRequired:
if tr is not None:
tr.close()
raise
except Exception:
if tr is not None:
tr.abort()
raise
state.write() state.write()
ui.progress(_("editing"), None) ui.progress(_("editing"), None)
def _finishhistedit(ui, repo, state): def _finishhistedit(ui, repo, state):
"""This action runs when histedit is finishing its session""" """This action runs when histedit is finishing its session"""
repo.ui.pushbuffer() repo.ui.pushbuffer()
hg.update(repo, state.parentctxnode, quietempty=True) hg.update(repo, state.parentctxnode, quietempty=True)
repo.ui.popbuffer() repo.ui.popbuffer()

mercurial/util.py

""" """
from __future__ import absolute_import from __future__ import absolute_import
import bz2 import bz2
import calendar import calendar
import codecs import codecs
import collections import collections
import contextlib
import datetime import datetime
import errno import errno
import gc import gc
import hashlib import hashlib
import imp import imp
import os import os
import platform as pyplatform import platform as pyplatform
import re as remod import re as remod
['b', 'a'] ['b', 'a']
''' '''
def __setitem__(self, key, value): def __setitem__(self, key, value):
if key in self: if key in self:
del self[key] del self[key]
super(sortdict, self).__setitem__(key, value) super(sortdict, self).__setitem__(key, value)
@contextlib.contextmanager
def acceptintervention(tr=None):
durhamUnsubmitted
Not Done

Technically this will apply to dsguard as well, so 'tr' isn't quite precise. But I think it's better to use 'tr' than invent some third term.

durham: Technically this will apply to dsguard as well, so 'tr' isn't quite precise. But I think it's…
"""A context manager that closes the transaction on InterventionRequired
If no transaction was provided, this simply runs the body and returns
"""
if not tr:
lothiraldanUnsubmitted
Not Done

The initial check was slighty more precise (if tr is not None), any specific reason for the change?

lothiraldan: The initial check was slighty more precise (if tr is not None), any specific reason for the…
martinvonzAuthorUnsubmitted
Not Done

We generally prefer "not X" over "is not None" when they're functionally equivalent (and I do think they are here).

martinvonz: We generally prefer "not X" over "is not None" when they're functionally equivalent (and I do…
yield
return
try:
yield
tr.close()
except error.InterventionRequired:
tr.close()
raise
finally:
tr.release()
class _lrucachenode(object): class _lrucachenode(object):
"""A node in a doubly linked list. """A node in a doubly linked list.
Holds a reference to nodes on either side as well as a key-value Holds a reference to nodes on either side as well as a key-value
pair for the dictionary entry. pair for the dictionary entry.
""" """
__slots__ = (u'next', u'prev', u'key', u'value') __slots__ = (u'next', u'prev', u'key', u'value')