D4827: context: move logic from changectx.__init__ to localrepo.__getitem__ (API)

[martinvonz (Martin von Zweigbergk)]

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

REVISION SUMMARY
  My motivation for this change was to make repo[node] not load the
  dirstate (more about that in the next patch), but I think it makes
  more sense this way too. For example, raising RepoLookupError seems to
  belong better in the repo lookup function
  (i.e. localrepo.__getitem__).
  
  This makes the changectx constructor very simple -- it just assigns
  the given repo, revnum, and nodeid to properties.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/context.py
  mercurial/localrepo.py

CHANGE DETAILS

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -17,8 +17,10 @@
 
 from .i18n import _
 from .node import (
+    bin,
     hex,
     nullid,
+    nullrev,
     short,
 )
 from . import (
@@ -1214,9 +1216,67 @@
                     for i in pycompat.xrange(*changeid.indices(len(self)))
                     if i not in self.changelog.filteredrevs]
         try:
-            return context.changectx(self, changeid)
+            if isinstance(changeid, int):
+                node = self.changelog.node(changeid)
+                rev = changeid
+                return context.changectx(self, rev, node)
+            elif changeid == 'null':
+                node = nullid
+                rev = nullrev
+                return context.changectx(self, rev, node)
+            elif changeid == 'tip':
+                node = self.changelog.tip()
+                rev = self.changelog.rev(node)
+                return context.changectx(self, rev, node)
+            elif (changeid == '.'
+                  or self.local() and changeid == self.dirstate.p1()):
+                # this is a hack to delay/avoid loading obsmarkers
+                # when we know that '.' won't be hidden
+                node = self.dirstate.p1()
+                rev = self.unfiltered().changelog.rev(node)
+                return context.changectx(self, rev, node)
+            elif len(changeid) == 20:
+                try:
+                    node = changeid
+                    rev = self.changelog.rev(changeid)
+                    return context.changectx(self, rev, node)
+                except error.FilteredLookupError:
+                    changeid = hex(changeid) # for the error message
+                    raise
+                except LookupError:
+                    # check if it might have come from damaged dirstate
+                    #
+                    # XXX we could avoid the unfiltered if we had a recognizable
+                    # exception for filtered changeset access
+                    if (self.local()
+                        and changeid in self.unfiltered().dirstate.parents()):
+                        msg = _("working directory has unknown parent '%s'!")
+                        raise error.Abort(msg % short(changeid))
+                    changeid = hex(changeid) # for the error message
+
+            elif len(changeid) == 40:
+                try:
+                    node = bin(changeid)
+                    rev = self.changelog.rev(node)
+                    return context.changectx(self, rev, node)
+                except error.FilteredLookupError:
+                    raise
+                except LookupError:
+                    pass
+            else:
+                raise error.ProgrammingError(
+                        "unsupported changeid '%s' of type %s" %
+                        (changeid, type(changeid)))
+
+        except (error.FilteredIndexError, error.FilteredLookupError):
+            raise error.FilteredRepoLookupError(_("filtered revision '%s'")
+                                                % pycompat.bytestr(changeid))
+        except IndexError:
+            pass
         except error.WdirUnsupported:
             return context.workingctx(self)
+        raise error.RepoLookupError(
+            _("unknown revision '%s'") % changeid)
 
     def __contains__(self, changeid):
         """True if the given changeid exists
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -15,7 +15,6 @@
 from .i18n import _
 from .node import (
     addednodeid,
-    bin,
     hex,
     modifiednodeid,
     nullid,
@@ -385,70 +384,11 @@
     """A changecontext object makes access to data related to a particular
     changeset convenient. It represents a read-only context already present in
     the repo."""
-    def __init__(self, repo, changeid='.'):
+    def __init__(self, repo, rev, node):
         """changeid is a revision number, node, or tag"""
         super(changectx, self).__init__(repo)
-
-        try:
-            if isinstance(changeid, int):
-                self._node = repo.changelog.node(changeid)
-                self._rev = changeid
-                return
-            elif changeid == 'null':
-                self._node = nullid
-                self._rev = nullrev
-                return
-            elif changeid == 'tip':
-                self._node = repo.changelog.tip()
-                self._rev = repo.changelog.rev(self._node)
-                return
-            elif (changeid == '.'
-                  or repo.local() and changeid == repo.dirstate.p1()):
-                # this is a hack to delay/avoid loading obsmarkers
-                # when we know that '.' won't be hidden
-                self._node = repo.dirstate.p1()
-                self._rev = repo.unfiltered().changelog.rev(self._node)
-                return
-            elif len(changeid) == 20:
-                try:
-                    self._node = changeid
-                    self._rev = repo.changelog.rev(changeid)
-                    return
-                except error.FilteredLookupError:
-                    changeid = hex(changeid) # for the error message
-                    raise
-                except LookupError:
-                    # check if it might have come from damaged dirstate
-                    #
-                    # XXX we could avoid the unfiltered if we had a recognizable
-                    # exception for filtered changeset access
-                    if (repo.local()
-                        and changeid in repo.unfiltered().dirstate.parents()):
-                        msg = _("working directory has unknown parent '%s'!")
-                        raise error.Abort(msg % short(changeid))
-                    changeid = hex(changeid) # for the error message
-
-            elif len(changeid) == 40:
-                try:
-                    self._node = bin(changeid)
-                    self._rev = repo.changelog.rev(self._node)
-                    return
-                except error.FilteredLookupError:
-                    raise
-                except LookupError:
-                    pass
-            else:
-                raise error.ProgrammingError(
-                        "unsupported changeid '%s' of type %s" %
-                        (changeid, type(changeid)))
-
-        except (error.FilteredIndexError, error.FilteredLookupError):
-            raise error.FilteredRepoLookupError(_("filtered revision '%s'")
-                                                % pycompat.bytestr(changeid))
-        except IndexError:
-            pass
-        raise error.RepoLookupError(
-            _("unknown revision '%s'") % changeid)
+        self._rev = rev
+        self._node = node
 
     def __hash__(self):
         try:



To: martinvonz, #hg-reviewers
Cc: mercurial-devel