Patchwork [01,of,35,V2] context: add an empty class that will be used as a parent of all contexts

login
register
mail settings
Submitter Sean Farley
Date Aug. 7, 2013, 11:51 p.m.
Message ID <339f5b17a04f405040de.1375919475@laptop.local>
Download mbox | patch
Permalink /patch/2054/
State Accepted
Headers show

Comments

Sean Farley - Aug. 7, 2013, 11:51 p.m.
# HG changeset patch
# User Sean Farley <sean.michael.farley@gmail.com>
# Date 1373763561 18000
#      Sat Jul 13 19:59:21 2013 -0500
# Node ID 339f5b17a04f405040de893becb51b7270b8613d
# Parent  df2155ebf502d14f7ef5276df806a35fc06dabd5
context: add an empty class that will be used as a parent of all contexts

At the moment, there is no simple way to check if an object is a context
because there is no common parent class. If there were, we could use
'isinstance' everywhere. Simply having memctx inherit from workingctx or
changectx would allow the use of 'isinstance' but that could lead to some
confusing situations of reading the code since we have three distinct concepts
of a context:

- changectx represents a changeset *already* in the repo, and is therefore immutable
- workingctx represents changes on disk in the working directory
- memctx represents changes solely in memory which may or may not be on disk

Therefore, I propose refactoring context.py to have all three contexts inherit
from a parent class 'context'.
Augie Fackler - Aug. 9, 2013, 3:16 p.m.
On Wed, Aug 07, 2013 at 06:51:15PM -0500, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean.michael.farley@gmail.com>
> # Date 1373763561 18000
> #      Sat Jul 13 19:59:21 2013 -0500
> # Node ID 339f5b17a04f405040de893becb51b7270b8613d
> # Parent  df2155ebf502d14f7ef5276df806a35fc06dabd5
> context: add an empty class that will be used as a parent of all contexts
>

Series LGTM. Anyone else want a look before I crew it?

>
> At the moment, there is no simple way to check if an object is a context
> because there is no common parent class. If there were, we could use
> 'isinstance' everywhere. Simply having memctx inherit from workingctx or
> changectx would allow the use of 'isinstance' but that could lead to some
> confusing situations of reading the code since we have three distinct concepts
> of a context:
>
> - changectx represents a changeset *already* in the repo, and is therefore immutable
> - workingctx represents changes on disk in the working directory
> - memctx represents changes solely in memory which may or may not be on disk
>
> Therefore, I propose refactoring context.py to have all three contexts inherit
> from a parent class 'context'.
>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -14,13 +14,24 @@
>  import obsolete as obsmod
>  import repoview
>
>  propertycache = util.propertycache
>
> -class changectx(object):
> +class context(object):
> +    """A context object represents the common logic for its children:
> +    changectx: read-only context that is already present in the repo,
> +    workingctx: a context that represents the working directory and can
> +                be committed,
> +    memctx: a context that represents changes in-memory and can also
> +            be committed."""
> +    def __new__(cls, repo, changeid='', *args, **kwargs):
> +        return super(context, cls).__new__(cls)
> +
> +class changectx(context):
>      """A changecontext object makes access to data related to a particular
> -    changeset convenient."""
> +    changeset convenient. It represents a read-only context already presnt in
> +    the repo."""
>      def __init__(self, repo, changeid=''):
>          """changeid is a revision number, node, or tag"""
>          if changeid == '':
>              changeid = '.'
>          self._repo = repo
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -14,13 +14,24 @@ 
 import obsolete as obsmod
 import repoview
 
 propertycache = util.propertycache
 
-class changectx(object):
+class context(object):
+    """A context object represents the common logic for its children:
+    changectx: read-only context that is already present in the repo,
+    workingctx: a context that represents the working directory and can
+                be committed,
+    memctx: a context that represents changes in-memory and can also
+            be committed."""
+    def __new__(cls, repo, changeid='', *args, **kwargs):
+        return super(context, cls).__new__(cls)
+
+class changectx(context):
     """A changecontext object makes access to data related to a particular
-    changeset convenient."""
+    changeset convenient. It represents a read-only context already presnt in
+    the repo."""
     def __init__(self, repo, changeid=''):
         """changeid is a revision number, node, or tag"""
         if changeid == '':
             changeid = '.'
         self._repo = repo