Patchwork D7406: scmutil: convert status data object from a tuple to an attrs (API)

login
register
mail settings
Submitter phabricator
Date Nov. 14, 2019, 9:12 p.m.
Message ID <differential-rev-PHID-DREV-nyype3aluxfwuhx7uviu-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/43210/
State Superseded
Headers show

Comments

phabricator - Nov. 14, 2019, 9:12 p.m.
durin42 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We've been pushing towards the property names for a while, and the
  subclassing of the tuple confuses pytype. Rather than bend over
  backwards to try and annotate the tuple subclass, let's just use attrs
  here.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/scmutil.py

CHANGE DETAILS




To: durin42, #hg-reviewers
Cc: mercurial-devel
phabricator - Nov. 15, 2019, 3:33 a.m.
This revision is now accepted and ready to land.
indygreg added a comment.
indygreg accepted this revision.


  Good riddance.
  
  Do the revlog index next! :)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7406/new/

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

To: durin42, #hg-reviewers, indygreg
Cc: indygreg, mercurial-devel
phabricator - Nov. 15, 2019, 5:50 a.m.
martinvonz added inline comments.

INLINE COMMENTS

> scmutil.py:81-88
> +    def __iter__(self):
> +        yield self.modified
> +        yield self.added
> +        yield self.removed
> +        yield self.deleted
> +        yield self.unknown
> +        yield self.ignored

What depends on this? It looked like many of the patches earlier in the series tried to get rid of iteration.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7406/new/

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

To: durin42, #hg-reviewers, indygreg
Cc: martinvonz, indygreg, mercurial-devel
phabricator - Nov. 15, 2019, 9:19 a.m.
Alphare added a comment.


  It appears PyCharm is confused by this change, which is weird, because it's supposed to support `attrs`. I would have to file a bug.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7406/new/

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

To: durin42, #hg-reviewers, indygreg
Cc: Alphare, martinvonz, indygreg, mercurial-devel
phabricator - Nov. 15, 2019, 5:27 p.m.
durin42 added inline comments.

INLINE COMMENTS

> martinvonz wrote in scmutil.py:81-88
> What depends on this? It looked like many of the patches earlier in the series tried to get rid of iteration.

I started out trying to get rid of the iterator interface on this, and gave up because it's just too many callsites for me to care.

I do think, long term, we should endeavor to rip this out.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7406/new/

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

To: durin42, #hg-reviewers, indygreg
Cc: Alphare, martinvonz, indygreg, mercurial-devel

Patch

diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -27,7 +27,7 @@ 
     wdirrev,
 )
 from .pycompat import getattr
-
+from .thirdparty import attr
 from . import (
     copies as copiesmod,
     encoding,
@@ -62,62 +62,36 @@ 
 termsize = scmplatform.termsize
 
 
-class status(tuple):
-    '''Named tuple with a list of files per status. The 'deleted', 'unknown'
-       and 'ignored' properties are only relevant to the working copy.
+@attr.s(slots=True, repr=False)
+class status(object):
+    '''Struct with a list of files per status.
+
+    The 'deleted', 'unknown' and 'ignored' properties are only
+    relevant to the working copy.
     '''
 
-    __slots__ = ()
-
-    def __new__(
-        cls, modified, added, removed, deleted, unknown, ignored, clean
-    ):
-        return tuple.__new__(
-            cls, (modified, added, removed, deleted, unknown, ignored, clean)
-        )
-
-    @property
-    def modified(self):
-        '''files that have been modified'''
-        return self[0]
-
-    @property
-    def added(self):
-        '''files that have been added'''
-        return self[1]
-
-    @property
-    def removed(self):
-        '''files that have been removed'''
-        return self[2]
+    modified = attr.ib(default=list)
+    added = attr.ib(default=list)
+    removed = attr.ib(default=list)
+    deleted = attr.ib(default=list)
+    unknown = attr.ib(default=list)
+    ignored = attr.ib(default=list)
+    clean = attr.ib(default=list)
 
-    @property
-    def deleted(self):
-        '''files that are in the dirstate, but have been deleted from the
-           working copy (aka "missing")
-        '''
-        return self[3]
-
-    @property
-    def unknown(self):
-        '''files not in the dirstate that are not ignored'''
-        return self[4]
+    def __iter__(self):
+        yield self.modified
+        yield self.added
+        yield self.removed
+        yield self.deleted
+        yield self.unknown
+        yield self.ignored
+        yield self.clean
 
-    @property
-    def ignored(self):
-        '''files not in the dirstate that are ignored (by _dirignore())'''
-        return self[5]
-
-    @property
-    def clean(self):
-        '''files that have not been modified'''
-        return self[6]
-
-    def __repr__(self, *args, **kwargs):
+    def __repr__(self):
         return (
-            r'<status modified=%s, added=%s, removed=%s, deleted=%s, '
-            r'unknown=%s, ignored=%s, clean=%s>'
-        ) % tuple(pycompat.sysstr(stringutil.pprint(v)) for v in self)
+            '<status modified=%r, added=%r, removed=%r, deleted=%r, '
+            + 'unknown=%r, ignored=%r, clean=%r>'
+        ) % tuple(self)
 
 
 def itersubrepos(ctx1, ctx2):