On 4/10/17 5:04 PM, Kostia Balytskyi wrote:
# HG changeset patch
# User Kostia Balytskyi <ikos...@fb.com>
# Date 1491840064 25200
# Mon Apr 10 09:01:04 2017 -0700
# Node ID 5dc11b1531f701844001b0723c4b8b97c2e55217
# Parent f6d77af84ef3e936b15634759df2718d5363b78a
shelve: refactor shelvestate loading
This is a preparatory patch which separates file reading from the
minimal validation we have (like turning version into int and
checking that this version is supported). The purpose of this patch
is to be able to read statefile form simplekeyvaluefile, which is
implemented in the following patch.
Nit: two empty lines in the description. Reduce to one please.
This series is some basic preparatory refactoring to make wrapping
parts of shelve easier in an extension.
diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -176,38 +176,44 @@ class shelvedstate(object):
@classmethod
def load(cls, repo):
+ # order is important, please do not change
Might be worth explaining *why* order is important.
+ keys = ['version', 'name', 'originalwctx', 'pendingctx', 'parents',
+ 'nodestoprune', 'branchtorestore', 'keep', 'activebook']
+ st = {}
"st" is not a great variable name. Can you call it "state" maybe?
fp = repo.vfs(cls._filename)
try:
- version = int(fp.readline().strip())
-
- if version != cls._version:
- raise error.Abort(_('this version of shelve is incompatible '
- 'with the version used in this repo'))
- name = fp.readline().strip()
- wctx = nodemod.bin(fp.readline().strip())
- pendingctx = nodemod.bin(fp.readline().strip())
- parents = [nodemod.bin(h) for h in fp.readline().split()]
- nodestoprune = [nodemod.bin(h) for h in fp.readline().split()]
- branchtorestore = fp.readline().strip()
- keep = fp.readline().strip() == cls._keep
- activebook = fp.readline().strip()
- except (ValueError, TypeError) as err:
- raise error.CorruptedState(str(err))
+ for key in keys:
+ st[key] = fp.readline().strip()
finally:
fp.close()
+ # some basic syntactic verification and transformation
+ try:
+ st['version'] = int(st.get('version'))
+ if st.get('version') != cls._version:
+ raise error.Abort(_('this version of shelve is incompatible '
+ 'with the version used in this repo'))
+ st['originalwctx'] = nodemod.bin(st.get('originalwctx'))
+ st['pendingctx'] = nodemod.bin(st.get('pendingctx'))
+ st['parents'] = [nodemod.bin(h)
+ for h in st.get('parents').split(' ')]
+ st['nodestoprune'] = [nodemod.bin(h)
+ for h in st.get('nodestoprune').split(' ')]
+ except (ValueError, TypeError, AttributeError) as err:
+ raise error.CorruptedState(str(err))
+
try:
obj = cls()
- obj.name = name
- obj.wctx = repo[wctx]
- obj.pendingctx = repo[pendingctx]
- obj.parents = parents
- obj.nodestoprune = nodestoprune
- obj.branchtorestore = branchtorestore
- obj.keep = keep
+ obj.name = st.get('name')
+ obj.wctx = repo[st.get('originalwctx')]
+ obj.pendingctx = repo[st.get('pendingctx')]
+ obj.parents = st.get('parents')
+ obj.nodestoprune = st.get('nodestoprune')
+ obj.branchtorestore = st.get('branchtorestore')
+ obj.keep = st.get('keep') == cls._keep
obj.activebookmark = ''
- if activebook != cls._noactivebook:
- obj.activebookmark = activebook
+ if st.get('activebook') != cls._noactivebook:
+ obj.activebookmark = st.get('activebook')
except error.RepoLookupError as err:
raise error.CorruptedState(str(err))
This looks like a correct mechanical change and a good improvement to
me. However, with the nitpicks and changes requested to other patches I
think it's worth one more iteration.
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
