This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7406

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

Authored by durin42 on Nov 14 2019, 4:09 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHGc5548b0b6847: scmutil: convert status data object from a tuple to an attrs (API)
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.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

durin42 created this revision.Nov 14 2019, 4:09 PM
durin42 updated this revision to Diff 18105.Nov 14 2019, 4:13 PM
indygreg accepted this revision.Nov 14 2019, 10:33 PM
indygreg added a subscriber: indygreg.

Good riddance.

Do the revlog index next! :)

This revision is now accepted and ready to land.Nov 14 2019, 10:33 PM
martinvonz added a subscriber: martinvonz.Nov 15 2019, 12:50 AM
martinvonz added inline comments.
mercurial/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.

Alphare added a subscriber: Alphare.Nov 15 2019, 4:19 AM

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.

durin42 added inline comments.Nov 15 2019, 12:27 PM
mercurial/scmutil.py
81–88

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.

Revision Contents
Changeset List

PathPackages
Mmercurial/scmutil.py (72 lines)

Diff 18134

mercurial/scmutil.py

hex, hex,
nullid, nullid,
nullrev, nullrev,
short, short,
wdirid, wdirid,
wdirrev, wdirrev,
) )
from .pycompat import getattr from .pycompat import getattr
from .thirdparty import attr
from . import ( from . import (
copies as copiesmod, copies as copiesmod,
encoding, encoding,
error, error,
match as matchmod, match as matchmod,
obsolete, obsolete,
obsutil, obsutil,
pathutil, pathutil,
else: else:
from . import scmposix as scmplatform from . import scmposix as scmplatform
parsers = policy.importmod('parsers') parsers = policy.importmod('parsers')
termsize = scmplatform.termsize termsize = scmplatform.termsize
class status(tuple): @attr.s(slots=True, repr=False)
'''Named tuple with a list of files per status. The 'deleted', 'unknown' class status(object):
and 'ignored' properties are only relevant to the working copy. '''Struct with a list of files per status.
'''
__slots__ = ()
def __new__(
cls, modified, added, removed, deleted, unknown, ignored, clean
):
return tuple.__new__(
cls, (modified, added, removed, deleted, unknown, ignored, clean)
)
@property The 'deleted', 'unknown' and 'ignored' properties are only
def modified(self): relevant to the working copy.
'''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]
@property
def deleted(self):
'''files that are in the dirstate, but have been deleted from the
working copy (aka "missing")
''' '''
return self[3]
@property modified = attr.ib(default=list)
def unknown(self): added = attr.ib(default=list)
'''files not in the dirstate that are not ignored''' removed = attr.ib(default=list)
return self[4] deleted = attr.ib(default=list)
unknown = attr.ib(default=list)
@property ignored = attr.ib(default=list)
def ignored(self): clean = attr.ib(default=list)
'''files not in the dirstate that are ignored (by _dirignore())'''
return self[5] def __iter__(self):
yield self.modified
@property yield self.added
def clean(self): yield self.removed
'''files that have not been modified''' yield self.deleted
return self[6] yield self.unknown
yield self.ignored
yield self.clean
martinvonzUnsubmitted
Not Done

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

martinvonz: What depends on this? It looked like many of the patches earlier in the series tried to get rid…
durin42AuthorUnsubmitted
Done

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.

durin42: I started out trying to get rid of the iterator interface on this, and gave up because it's…
def __repr__(self, *args, **kwargs): def __repr__(self):
return ( return (
r'<status modified=%s, added=%s, removed=%s, deleted=%s, ' r'<status modified=%s, added=%s, removed=%s, deleted=%s, '
r'unknown=%s, ignored=%s, clean=%s>' r'unknown=%s, ignored=%s, clean=%s>'
) % tuple(pycompat.sysstr(stringutil.pprint(v)) for v in self) ) % tuple(pycompat.sysstr(stringutil.pprint(v)) for v in self)
def itersubrepos(ctx1, ctx2): def itersubrepos(ctx1, ctx2):
"""find subrepos in ctx1 or ctx2""" """find subrepos in ctx1 or ctx2"""