[PATCH 6 of 8] dirstate: create class for status lists
Martin von Zweigbergk
martinvonz at gmail.com
Thu Oct 2 00:00:15 CDT 2014
Thanks for taking a look! Answers below to some of your questions.
I'll leave the others for later, or they might just get address in v2
of the series.
On Wed, Oct 1, 2014 at 6:40 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
> Comments added all over. I choose to send it now instead of polishing it and
> perhaps sending it tomorrow ;-)
>
>
> On 10/02/2014 02:01 AM, Martin von Zweigbergk wrote:
>>
>> # HG changeset patch
>> # User Martin von Zweigbergk <martinvonz at gmail.com>
>> # Date 1412204534 25200
>> # Wed Oct 01 16:02:14 2014 -0700
>> # Node ID 048f3810ef38a85db4b265f020fe94fe6a12d103
>> # Parent 31cbb21f1283847c65e385bd94b81e2c78fd5121
>> dirstate: create class for status lists
>
>
> I'm spoiled by using Skype - that influence my vocabulary. I just want to
> say: (heart)
>
>> Callers of various status() methods (on dirstate, context, repo) get a
>> tuple of 7 or 8 elements, where each element is a list of files. This
>> results in lots of uses of indexes where names would be much more
>> readable. For example, "status.ignored" seems clearer than "status[4]"
>> [1]. So, let's introduce a simple class containing the 7 status
>> fields: modified, added, removed, deleted, unknown, ignored, clean. In
>> the cases of dirstate.status(), which has an 8th element ('lookup'),
>> return a 2-tuple of the new status class and the lookup list.
>>
>> Having a class for the status also lets us add methods to it. For
>> example, a status.haschanges() method turns out to be useful.
>>
>>
>> [1] Did you even notice that it should have been "status[5]"?
>
>
> ;-)
>
>
> A general comment: A "." lookup is relatively expensive. It seems like you
> introduce a lot of them. We should take care to not introduce more of them
> in tight loops (that might already be the case - I did not check and not see
> any statement on whether that was the case).
Hmm, I don't think I even considered that (I've never used python before).
I tried running the test suite:
Before:
real 3m22.026s
user 25m31.715s
sys 5m45.684s
After:
real 3m24.136s
user 25m29.724s
sys 5m46.637s
I also tried this:
$ python -mtimeit -s'a=(1,2,3,4,5,6)' 'a[3]'
10000000 loops, best of 3: 0.0402 usec per loop
$ python -mtimeit -s'class c(object):
def __init__(self,a,b,c,d,e,f):
self.a=a
self.b=b
self.c=c
self.d=d
self.e=e
self.f=f
a=c(1,2,3,4,5,6)' 'a.d'
10000000 loops, best of 3: 0.0458 usec per loop
So no big difference in either test. I'm running Python 2.7.3. Is that
small difference still significant overall, you think? Or was the
performance penalty just there in older Pythons? (And do we care in
that case?) Or am I not testing it right?
> I also think you should
> consider make the diff smaller by starting out with assigning the values to
> the old local variables (you already do that in some places but it could
> perhaps be done in more).
I think that makes sense. I'll give it a shot.
>
>> diff --git a/hgext/convert/hg.py b/hgext/convert/hg.py
>> --- a/hgext/convert/hg.py
>> +++ b/hgext/convert/hg.py
>> @@ -371,7 +371,8 @@
>> if self._changescache[0] == rev:
>> m, a, r = self._changescache[1]
>> else:
>> - m, a, r = self.repo.status(parents[0].node(),
>> ctx.node())[:3]
>> + s = self.repo.status(parents[0].node(), ctx.node())
>> + m, a, r = s.modified, s.added, s.removed
>> if not full:
>> files = m + a + r
>> copyfiles = m + a
>> @@ -437,7 +438,8 @@
>> changes = [], ctx.manifest().keys(), []
>> else:
>> i = i or 0
>> - changes = self.repo.status(parents[i].node(), ctx.node())[:3]
>> + s = self.repo.status(parents[i].node(), ctx.node())
>> + changes = s.modified, s.added, s.removed
>> changes = [[f for f in l if f not in self.ignored] for l in
>> changes]
>> if i == 0:
>> diff --git a/hgext/extdiff.py b/hgext/extdiff.py
>> --- a/hgext/extdiff.py
>> +++ b/hgext/extdiff.py
>> @@ -143,9 +143,12 @@
>> do3way = False
>> matcher = scmutil.match(repo[node2], pats, opts)
>> - mod_a, add_a, rem_a = map(set, repo.status(node1a, node2,
>> matcher)[:3])
>> + st_a = repo.status(node1a, node2, matcher)
>> + mod_a, add_a, rem_a = map(set, (st_a.modified, st_a.added,
>> st_a.removed))
>
>
> This map do no longer make the code shorter or more readable.
>
>> if do3way:
>> - mod_b, add_b, rem_b = map(set, repo.status(node1b, node2,
>> matcher)[:3])
>> + st_b = repo.status(node1b, node2, matcher)
>> + mod_b, add_b, rem_b = map(set,
>> + (st_b.modified, st_b.added,
>> st_b.removed))
>> else:
>> mod_b, add_b, rem_b = set(), set(), set()
>> modadd = mod_a | add_a | mod_b | add_b
>> diff --git a/hgext/gpg.py b/hgext/gpg.py
>> --- a/hgext/gpg.py
>> +++ b/hgext/gpg.py
>> @@ -255,7 +255,7 @@
>> msigs = match.exact(repo.root, '', ['.hgsigs'])
>> s = repo.status(match=msigs, unknown=True, ignored=True)
>> - if util.any(s) and not opts["force"]:
>> + if util.any(s.all()) and not opts["force"]:
>> raise util.Abort(_("working copy of .hgsigs is changed "
>> "(please commit .hgsigs manually "
>> "or use --force)"))
>> diff --git a/hgext/hgcia.py b/hgext/hgcia.py
>> --- a/hgext/hgcia.py
>> +++ b/hgext/hgcia.py
>> @@ -82,14 +82,14 @@
>> if url and url[-1] == '/':
>> url = url[:-1]
>> elems = []
>> - for path in f[0]:
>> + for path in f.modified:
>> uri = '%s/diff/%s/%s' % (url, short(n), path)
>> elems.append(self.fileelem(path, url and uri, 'modify'))
>> - for path in f[1]:
>> + for path in f.added:
>> # TODO: copy/rename ?
>> uri = '%s/file/%s/%s' % (url, short(n), path)
>> elems.append(self.fileelem(path, url and uri, 'add'))
>> - for path in f[2]:
>> + for path in f.removed:
>> elems.append(self.fileelem(path, '', 'remove'))
>> return '\n'.join(elems)
>> diff --git a/hgext/histedit.py b/hgext/histedit.py
>> --- a/hgext/histedit.py
>> +++ b/hgext/histedit.py
>> @@ -688,8 +688,7 @@
>> # Commit dirty working directory if necessary
>> new = None
>> - m, a, r, d = repo.status()[:4]
>> - if m or a or r or d:
>> + if repo.status().haschanges():
>> # prepare the message for the commit to comes
>> if action in ('f', 'fold', 'r', 'roll'):
>> message = 'fold-temp-revision %s' % currentnode
>> diff --git a/hgext/keyword.py b/hgext/keyword.py
>> --- a/hgext/keyword.py
>> +++ b/hgext/keyword.py
>> @@ -171,9 +171,8 @@
>> '''Retrieves modified and added files from a working directory state
>> and returns the subset of each contained in given changed files
>> retrieved from a change context.'''
>> - modified, added = wstatus[:2]
>> - modified = [f for f in modified if f in changed]
>> - added = [f for f in added if f in changed]
>> + modified = [f for f in wstatus.modified if f in changed]
>> + added = [f for f in wstatus.added if f in changed]
>> return modified, added
>> @@ -349,10 +348,9 @@
>> wlock = repo.wlock()
>> try:
>> status = _status(ui, repo, wctx, kwt, *pats, **opts)
>> - modified, added, removed, deleted, unknown, ignored, clean =
>> status
>> - if modified or added or removed or deleted:
>> + if status.haschanges():
>> raise util.Abort(_('outstanding uncommitted changes'))
>> - kwt.overwrite(wctx, clean, True, expand)
>> + kwt.overwrite(wctx, status.clean, True, expand)
>> finally:
>> wlock.release()
>> @@ -503,20 +501,19 @@
>> wctx = repo[None]
>> status = _status(ui, repo, wctx, kwt, *pats, **opts)
>> cwd = pats and repo.getcwd() or ''
>> - modified, added, removed, deleted, unknown, ignored, clean = status
>> files = []
>> if not opts.get('unknown') or opts.get('all'):
>> - files = sorted(modified + added + clean)
>> + files = sorted(status.modified + status.added + status.clean)
>> kwfiles = kwt.iskwfile(files, wctx)
>> - kwdeleted = kwt.iskwfile(deleted, wctx)
>> - kwunknown = kwt.iskwfile(unknown, wctx)
>> + kwdeleted = kwt.iskwfile(status.deleted, wctx)
>> + kwunknown = kwt.iskwfile(status.unknown, wctx)
>> if not opts.get('ignore') or opts.get('all'):
>> showfiles = kwfiles, kwdeleted, kwunknown
>> else:
>> showfiles = [], [], []
>> if opts.get('all') or opts.get('ignore'):
>> showfiles += ([f for f in files if f not in kwfiles],
>> - [f for f in unknown if f not in kwunknown])
>> + [f for f in status.unknown if f not in kwunknown])
>> kwlabels = 'enabled deleted enabledunknown ignored
>> ignoredunknown'.split()
>> kwstates = zip(kwlabels, 'K!kIi', showfiles)
>> fm = ui.formatter('kwfiles', opts)
>> diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py
>> --- a/hgext/largefiles/lfutil.py
>> +++ b/hgext/largefiles/lfutil.py
>> @@ -136,19 +136,18 @@
>> def lfdirstatestatus(lfdirstate, repo, rev):
>> match = match_.always(repo.root, repo.getcwd())
>> - s = lfdirstate.status(match, [], False, False, False)
>> - unsure, modified, added, removed, missing, unknown, ignored, clean =
>> s
>> - for lfile in unsure:
>> + (s, lookup) = lfdirstate.status(match, [], False, False, False)
>> + for lfile in lookup:
>
>
> I don't think I like the renaming from unsure to lookup. Unsure describes
> the content of the list. Lookup is what we might want to do with it because
> of that.
I asked on IRC and mpm preferred 'lookup' because it's older, IIRC.
There's also the risk of confusing 'unsure' and 'unknown'. I can see
how you would prefer 'unsure', though. I did, at least initially, but
I picked the other name for some reason.
> Anyway, it seems like an unrelated change that could be done (and discussed)
> later.
True, it's definitely a separate change now (see next answer). Will extract.
> I also wonder if it would make sense to make "unsure" an optional member of
> the status struct. I don't know.
I know, and it doesn't :-). I did that at first, but it just got ugly
with having to assert (or assume) that lookup/unsure was empty where
it should be.
>
>> try:
>> fctx = repo[rev][standin(lfile)]
>> except LookupError:
>> fctx = None
>> if not fctx or fctx.data().strip() !=
>> hashfile(repo.wjoin(lfile)):
>> - modified.append(lfile)
>> + s.modified.append(lfile)
>> else:
>> - clean.append(lfile)
>> + s.clean.append(lfile)
>> lfdirstate.normal(lfile)
>> - return (modified, added, removed, missing, unknown, ignored, clean)
>> + return s
>> def listlfiles(repo, rev=None, matcher=None):
>> '''return a list of largefiles in the working copy or the
>> diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
>> --- a/hgext/largefiles/overrides.py
>> +++ b/hgext/largefiles/overrides.py
>> @@ -153,7 +153,8 @@
>> manifest = repo[None].manifest()
>> modified, added, deleted, clean = [[f for f in list
>> if lfutil.standin(f) in
>> manifest]
>> - for list in [s[0], s[1], s[3],
>> s[6]]]
>> + for list in (s.modified, s.added,
>> + s.deleted, s.clean)]
>> def warn(files, msg):
>> for f in files:
>> @@ -353,13 +354,11 @@
>> wlock = repo.wlock()
>> try:
>> lfdirstate = lfutil.openlfdirstate(ui, repo)
>> - s = lfdirstate.status(match_.always(repo.root, repo.getcwd()),
>> - [], False, False, False)
>> - (unsure, modified, added, removed, missing, unknown, ignored,
>> clean) = s
>> -
>> + (s, lookup) = lfdirstate.status(match_.always(repo.root,
>> repo.getcwd()),
>> + [], False, False, False)
>> if opts['check']:
>> - mod = len(modified) > 0
>> - for lfile in unsure:
>> + mod = len(s.modified) > 0
>> + for lfile in lookup:
>> standin = lfutil.standin(lfile)
>> if repo['.'][standin].data().strip() != \
>> lfutil.hashfile(repo.wjoin(lfile)):
>> @@ -662,12 +661,11 @@
>> wlock = repo.wlock()
>> try:
>> lfdirstate = lfutil.openlfdirstate(ui, repo)
>> - (modified, added, removed, missing, unknown, ignored, clean) = \
>> - lfutil.lfdirstatestatus(lfdirstate, repo, repo['.'].rev())
>> + s = lfutil.lfdirstatestatus(lfdirstate, repo, repo['.'].rev())
>> lfdirstate.write()
>> - for lfile in modified:
>> + for lfile in s.modified:
>> lfutil.updatestandin(repo, lfutil.standin(lfile))
>> - for lfile in missing:
>> + for lfile in s.deleted:
>
>
> The change would be easier to review if missing was renamed to deleted in a
> separate changeset. It might however not worth the trouble ...
>
>> if (os.path.exists(repo.wjoin(lfutil.standin(lfile)))):
>> os.unlink(repo.wjoin(lfutil.standin(lfile)))
>> @@ -956,17 +954,15 @@
>> def overridebailifchanged(orig, repo):
>> orig(repo)
>> repo.lfstatus = True
>> - modified, added, removed, deleted = repo.status()[:4]
>> repo.lfstatus = False
>> - if modified or added or removed or deleted:
>> + if repo.status().haschanges():
>> raise util.Abort(_('uncommitted changes'))
>> # Fetch doesn't use cmdutil.bailifchanged so override it to add the
>> check
>> def overridefetch(orig, ui, repo, *pats, **opts):
>> repo.lfstatus = True
>> - modified, added, removed, deleted = repo.status()[:4]
>> repo.lfstatus = False
>> - if modified or added or removed or deleted:
>> + if repo.status().haschanges():
>> raise util.Abort(_('uncommitted changes'))
>> return orig(ui, repo, *pats, **opts)
>> @@ -981,7 +977,7 @@
>> s = repo.status(match=m, clean=True)
>> finally:
>> repo.lfstatus = False
>> - forget = sorted(s[0] + s[1] + s[3] + s[6])
>> + forget = sorted(s.modified + s.added + s.deleted + s.clean)
>> forget = [f for f in forget if lfutil.standin(f) in
>> repo[None].manifest()]
>> for f in forget:
>> @@ -1112,16 +1108,15 @@
>> return orig(repo, pats, opts, dry_run, similarity)
>> # Get the list of missing largefiles so we can remove them
>> lfdirstate = lfutil.openlfdirstate(repo.ui, repo)
>> - s = lfdirstate.status(match_.always(repo.root, repo.getcwd()), [],
>> False,
>> - False, False)
>> - (unsure, modified, added, removed, missing, unknown, ignored, clean)
>> = s
>> + (s, lookup) = lfdirstate.status(match_.always(repo.root,
>> repo.getcwd()), [],
>> + False, False, False)
>> # Call into the normal remove code, but the removing of the
>> standin, we want
>> # to have handled by original addremove. Monkey patching here makes
>> sure
>> # we don't remove the standin in the largefiles code, preventing a
>> very
>> # confused state later.
>> - if missing:
>> - m = [repo.wjoin(f) for f in missing]
>> + if s.deleted:
>> + m = [repo.wjoin(f) for f in s.deleted]
>> repo._isaddremove = True
>> removelargefiles(repo.ui, repo, *m, **opts)
>> repo._isaddremove = False
>> @@ -1148,10 +1143,9 @@
>> r = oldstatus(node1, node2, match, ignored, clean, unknown,
>> listsubrepos)
>> lfdirstate = lfutil.openlfdirstate(ui, repo)
>> - modified, added, removed, deleted, unknown, ignored, clean = r
>> - unknown = [f for f in unknown if lfdirstate[f] == '?']
>> - ignored = [f for f in ignored if lfdirstate[f] == '?']
>> - return modified, added, removed, deleted, unknown, ignored, clean
>> + r.unknown = [f for f in r.unknown if lfdirstate[f] == '?']
>> + r.ignored = [f for f in r.ignored if lfdirstate[f] == '?']
>> + return r
>> repo.status = overridestatus
>> orig(ui, repo, *dirs, **opts)
>> repo.status = oldstatus
>> @@ -1290,10 +1284,10 @@
>> # update standins for linear-merge or force-branch-merge,
>> # because largefiles in the working directory may be
>> modified
>> lfdirstate = lfutil.openlfdirstate(repo.ui, repo)
>> - s = lfdirstate.status(match_.always(repo.root,
>> repo.getcwd()),
>> - [], False, False, False)
>> - unsure, modified, added = s[:3]
>> - for lfile in unsure + modified + added:
>> + (s, lookup) = lfdirstate.status(match_.always(repo.root,
>> + repo.getcwd()),
>> + [], False, False, False)
>> + for lfile in lookup + s.modified + s.added:
>> lfutil.updatestandin(repo, lfutil.standin(lfile))
>> if linearmerge:
>> diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
>> --- a/hgext/largefiles/reposetup.py
>> +++ b/hgext/largefiles/reposetup.py
>> @@ -5,6 +5,7 @@
>> #
>> # This software may be used and distributed according to the terms of
>> the
>> # GNU General Public License version 2 or any later version.
>> +from mercurial.dirstate import status
>
>
> It is better to use from mercurial import dirstate and then use
> dirstate.status later on. Both to preserve the namespacing but also to allow
> demandimport to do its job.
>
>> '''setup for largefiles repositories: reposetup'''
>> import copy
>> @@ -159,12 +160,13 @@
>> if sfindirstate(f)]
>> # Don't waste time getting the ignored and unknown
>> # files from lfdirstate
>> - s = lfdirstate.status(match, [], False,
>> - listclean, False)
>> - (unsure, modified, added, removed, missing, _unknown,
>> - _ignored, clean) = s
>> + (s, lookup) = lfdirstate.status(match, [], False,
>> + listclean, False)
>> + modified = s.modified
>> + added = s.added
>> + clean = s.clean
>> if parentworking:
>> - for lfile in unsure:
>> + for lfile in lookup:
>> standin = lfutil.standin(lfile)
>> if standin not in ctx1:
>> # from second parent
>> @@ -177,7 +179,7 @@
>> clean.append(lfile)
>> lfdirstate.normal(lfile)
>> else:
>> - tocheck = unsure + modified + added + clean
>> + tocheck = lookup + s.modified + s.added + s.clean
>> modified, added, clean = [], [], []
>> for lfile in tocheck:
>> @@ -191,6 +193,7 @@
>> else:
>> added.append(lfile)
>> + removed = s.removed
>> # Standins no longer found in lfdirstate has been
>> # removed
>> for standin in
>> ctx1.walk(lfutil.getstandinmatcher(self)):
>> @@ -200,9 +203,6 @@
>> if lfile not in lfdirstate:
>> removed.append(lfile)
>> - # Filter result lists
>> - result = list(result)
>> -
>> # Largefiles are not really removed when they're
>> # still in the normal dirstate. Likewise, normal
>> # files are not really removed if they are still in
>> @@ -210,19 +210,19 @@
>> # change type.
>> removed = [f for f in removed
>> if f not in self.dirstate]
>> - result[2] = [f for f in result[2]
>> + result.removed = [f for f in result.removed
>> if f not in lfdirstate]
>> lfiles = set(lfdirstate._map)
>> # Unknown files
>> - result[4] = set(result[4]).difference(lfiles)
>> + result.unknown =
>> set(result.unknown).difference(lfiles)
>> # Ignored files
>> - result[5] = set(result[5]).difference(lfiles)
>> + result.ignored =
>> set(result.ignored).difference(lfiles)
>> # combine normal files and largefiles
>> normals = [[fn for fn in filelist
>> if not lfutil.isstandin(fn)]
>> - for filelist in result]
>> - lfstatus = (modified, added, removed, missing, [],
>> [],
>> + for filelist in result.all()]
>> + lfstatus = (modified, added, removed, s.deleted, [],
>> [],
>> clean)
>> result = [sorted(list1 + list2)
>> for (list1, list2) in zip(normals,
>> lfstatus)]
>> @@ -234,6 +234,8 @@
>> result = [[toname(f) for f in items]
>> for items in result]
>> + result = status(result[0], result[1], result[2],
>> result[3],
>> + result[4], result[5], result[6])
>> if wlock:
>> lfdirstate.write()
>> @@ -296,10 +298,9 @@
>> # large.
>> lfdirstate = lfutil.openlfdirstate(ui, self)
>> dirtymatch = match_.always(self.root, self.getcwd())
>> - s = lfdirstate.status(dirtymatch, [], False, False,
>> False)
>> - (unsure, modified, added, removed, _missing,
>> _unknown,
>> - _ignored, _clean) = s
>> - modifiedfiles = unsure + modified + added + removed
>> + (s, lookup) = lfdirstate.status(dirtymatch, [],
>> False,
>> + False, False)
>> + modifiedfiles = lookup + s.modified + s.added +
>> s.removed
>> lfiles = lfutil.listlfiles(self)
>> # this only loops through largefiles that exist (not
>> # removed/renamed)
>> diff --git a/hgext/mq.py b/hgext/mq.py
>> --- a/hgext/mq.py
>> +++ b/hgext/mq.py
>> @@ -1021,7 +1021,7 @@
>> return None, None
>> def putsubstate2changes(self, substatestate, changes):
>> - for files in changes[:3]:
>> + for files in changes:
>
>
> This imply a change from a list of lists to a single list. Could that be
> done in a separate changeset?
>
>> if '.hgsubstate' in files:
>> return # already listed up
>> # not yet listed up
>> @@ -1092,11 +1092,11 @@
>> if f != '.hgsubstate': # .hgsubstate is auto-created
>> raise util.Abort('%s: %s' % (f, msg))
>> match.bad = badfn
>> - changes = repo.status(match=match)
>> + changes = repo.status(match=match).committable()
>> else:
>> - changes = self.checklocalchanges(repo, force=True)
>> + changes = self.checklocalchanges(repo,
>> force=True).committable()
>> commitfiles = list(inclsubs)
>> - for files in changes[:3]:
>> + for files in changes:
>> commitfiles.extend(files)
>> match = scmutil.matchfiles(repo, commitfiles)
>> if len(repo[None].parents()) > 1:
>> @@ -1360,11 +1360,12 @@
>> tobackup = set()
>> if (not nobackup and force) or keepchanges:
>> - m, a, r, d = self.checklocalchanges(repo, force=True)
>> + changes = self.checklocalchanges(repo, force=True)
>> if keepchanges:
>> - tobackup.update(m + a + r + d)
>> + tobackup.update(changes.modified + changes.added +
>> + changes.removed + changes.deleted)
>> else:
>> - tobackup.update(m + a)
>> + tobackup.update(changes.modified + changes.added)
>> s = self.series[start:end]
>> all_files = set()
>> @@ -1448,13 +1449,14 @@
>> tobackup = set()
>> if update:
>> - m, a, r, d = self.checklocalchanges(
>> + changes = self.checklocalchanges(
>> repo, force=force or keepchanges)
>> if force:
>> if not nobackup:
>> - tobackup.update(m + a)
>> + tobackup.update(changes.modified + changes.added)
>> elif keepchanges:
>> - tobackup.update(m + a + r + d)
>> + tobackup.update(changes.modified + changes.added +
>> + changes.removed + changes.deleted)
>> self.applieddirty = True
>> end = len(self.applied)
>> @@ -1479,19 +1481,20 @@
>> if update:
>> qp = self.qparents(repo, rev)
>> ctx = repo[qp]
>> - m, a, r, d = repo.status(qp, '.')[:4]
>> - if d:
>> + status = repo.status(qp, '.')
>> + if status.deleted:
>> raise util.Abort(_("deletions found between repo
>> revs"))
>> - tobackup = set(a + m + r) & tobackup
>> + tobackup = set(status.added + status.modified +
>> + status.removed) & tobackup
>> if keepchanges and tobackup:
>> raise util.Abort(_("local changes found, refresh
>> first"))
>> self.backup(repo, tobackup)
>> repo.dirstate.beginparentchange()
>> - for f in a:
>> + for f in status.added:
>> util.unlinkpath(repo.wjoin(f), ignoremissing=True)
>> repo.dirstate.drop(f)
>> - for f in m + r:
>> + for f in status.modified + status.removed:
>> fctx = ctx[f]
>> repo.wwrite(f, fctx.data(), fctx.flags())
>> repo.dirstate.normal(f)
>> @@ -1570,7 +1573,8 @@
>> # mm, dd, aa = repo.status(top, patchparent)[:3]
>> # but we do it backwards to take advantage of
>> manifest/changelog
>> # caching against the next repo.status call
>> - mm, aa, dd = repo.status(patchparent, top)[:3]
>> + s = repo.status(patchparent, top)
>> + mm, aa, dd = s.modified, s.added, s.removed
>> changes = repo.changelog.read(top)
>> man = repo.manifest.read(changes[0])
>> aaa = aa[:]
>> @@ -1585,7 +1589,8 @@
>> matchfn = scmutil.match(repo[None], opts=opts)
>> else:
>> match = scmutil.matchall(repo)
>> - m, a, r, d = repo.status(match=match)[:4]
>> + s = repo.status(match=match)
>> + m, a, r, d = s.modified, s.added, s.removed, s.deleted
>> mm = set(mm)
>> aa = set(aa)
>> dd = set(dd)
>> @@ -1624,8 +1629,9 @@
>> # create 'match' that includes the files to be
>> recommitted.
>> # apply matchfn via repo.status to ensure correct case
>> handling.
>> - cm, ca, cr, cd = repo.status(patchparent, match=matchfn)[:4]
>> - allmatches = set(cm + ca + cr + cd)
>> + status = repo.status(patchparent, match=matchfn)
>> + allmatches = set(status.modified + status.added +
>> status.removed +
>> + status.deleted)
>> refreshchanges = [x.intersection(allmatches) for x in (mm,
>> aa, dd)]
>> files = set(inclsubs)
>> diff --git a/hgext/purge.py b/hgext/purge.py
>> --- a/hgext/purge.py
>> +++ b/hgext/purge.py
>> @@ -102,7 +102,7 @@
>> status = repo.status(match=match, ignored=opts['all'], unknown=True)
>> if removefiles:
>> - for f in sorted(status[4] + status[5]):
>> + for f in sorted(status.unknown + status.ignored):
>> if act:
>> ui.note(_('removing file %s\n') % f)
>> remove(util.unlink, f)
>> diff --git a/hgext/record.py b/hgext/record.py
>> --- a/hgext/record.py
>> +++ b/hgext/record.py
>> @@ -519,12 +519,12 @@
>> raise util.Abort(_('cannot partially commit a merge '
>> '(use "hg commit" instead)'))
>> - changes = repo.status(match=match)[:3]
>> + status = repo.status(match=match)
>> diffopts = opts.copy()
>> diffopts['nodates'] = True
>> diffopts['git'] = True
>> diffopts = patch.diffopts(ui, opts=diffopts)
>> - chunks = patch.diff(repo, changes=changes, opts=diffopts)
>> + chunks = patch.diff(repo, changes=status.committable(),
>> opts=diffopts)
>> fp = cStringIO.StringIO()
>> fp.write(''.join(chunks))
>> fp.seek(0)
>> @@ -544,13 +544,13 @@
>> except AttributeError:
>> pass
>> - changed = changes[0] + changes[1] + changes[2]
>> + changed = status.modified + status.added + status.removed
>> newfiles = [f for f in changed if f in contenders]
>> if not newfiles:
>> ui.status(_('no changes to record\n'))
>> return 0
>> - modified = set(changes[0])
>> + modified = set(status.modified)
>> # 2. backup changed files, so we can restore them in the end
>> if backupall:
>> diff --git a/hgext/shelve.py b/hgext/shelve.py
>> --- a/hgext/shelve.py
>> +++ b/hgext/shelve.py
>> @@ -227,9 +227,9 @@
>> if not node:
>> stat = repo.status(match=scmutil.match(repo[None], pats,
>> opts))
>> - if stat[3]:
>> + if stat.deleted:
>> ui.status(_("nothing changed (%d missing files, see "
>> - "'hg status')\n") % len(stat[3]))
>> + "'hg status')\n") % len(stat.deleted))
>> else:
>> ui.status(_("nothing changed\n"))
>> return 1
>> @@ -401,7 +401,7 @@
>> files.extend(shelvectx.parents()[0].files())
>> # revert will overwrite unknown files, so move them out of the
>> way
>> - for file in repo.status(unknown=True)[4]:
>> + for file in repo.status(unknown=True).unknown:
>> if file in files:
>> util.rename(file, file + ".orig")
>> ui.pushbuffer(True)
>> @@ -545,8 +545,7 @@
>> # to the original pctx.
>> # Store pending changes in a commit
>> - m, a, r, d = repo.status()[:4]
>> - if m or a or r or d:
>> + if repo.status().haschanges():
>> ui.status(_("temporarily committing pending changes "
>> "(restore with 'hg unshelve --abort')\n"))
>> def commitfunc(ui, repo, message, match, opts):
>> diff --git a/hgext/strip.py b/hgext/strip.py
>> --- a/hgext/strip.py
>> +++ b/hgext/strip.py
>> @@ -32,15 +32,15 @@
>> def checklocalchanges(repo, force=False, excsuffix=''):
>> cmdutil.checkunfinished(repo)
>> - m, a, r, d = repo.status()[:4]
>> + status = repo.status()
>> if not force:
>> - if (m or a or r or d):
>> + if (status.haschanges()):
>> _("local changes found") # i18n tool detection
>> raise util.Abort(_("local changes found" + excsuffix))
>> if checksubstate(repo):
>> _("local changed subrepos found") # i18n tool detection
>> raise util.Abort(_("local changed subrepos found" +
>> excsuffix))
>> - return m, a, r, d
>> + return status
>> def strip(ui, repo, revs, update=True, backup=True, force=None,
>> bookmark=None):
>> wlock = lock = None
>> diff --git a/hgext/transplant.py b/hgext/transplant.py
>> --- a/hgext/transplant.py
>> +++ b/hgext/transplant.py
>> @@ -611,8 +611,7 @@
>> if not opts.get('continue'):
>> if p2 != revlog.nullid:
>> raise util.Abort(_('outstanding uncommitted merges'))
>> - m, a, r, d = repo.status()[:4]
>> - if m or a or r or d:
>> + if repo.status().haschanges():
>> raise util.Abort(_('outstanding local changes'))
>> sourcerepo = opts.get('source')
>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>> --- a/mercurial/cmdutil.py
>> +++ b/mercurial/cmdutil.py
>> @@ -83,8 +83,7 @@
>> def bailifchanged(repo):
>> if repo.dirstate.p2() != nullid:
>> raise util.Abort(_('outstanding uncommitted merge'))
>> - modified, added, removed, deleted = repo.status()[:4]
>> - if modified or added or removed or deleted:
>> + if repo.status().haschanges():
>> raise util.Abort(_('uncommitted changes'))
>> ctx = repo[None]
>> for s in sorted(ctx.substate):
>> @@ -940,7 +939,8 @@
>> label='log.date')
>> if self.ui.debugflag:
>> - files = self.repo.status(log.parents(changenode)[0],
>> changenode)[:3]
>> + status = self.repo.status(log.parents(changenode)[0],
>> changenode)
>> + files = (status.modified, status.added, status.removed)
>> for key, value in zip([# i18n: column positioning for "hg
>> log"
>> _("files:"),
>> # i18n: column positioning for "hg
>> log"
>> @@ -1074,11 +1074,11 @@
>> files = ctx.status(ctx.p1())
>> self.ui.write(',\n "modified": [%s]' %
>> - ", ".join('"%s"' % j(f) for f in files[0]))
>> + ", ".join('"%s"' % j(f) for f in
>> files.modified))
>> self.ui.write(',\n "added": [%s]' %
>> - ", ".join('"%s"' % j(f) for f in files[1]))
>> + ", ".join('"%s"' % j(f) for f in files.added))
>> self.ui.write(',\n "removed": [%s]' %
>> - ", ".join('"%s"' % j(f) for f in files[2]))
>> + ", ".join('"%s"' % j(f) for f in
>> files.removed))
>> elif self.ui.verbose:
>> self.ui.write(',\n "files": [%s]' %
>> @@ -2016,7 +2016,7 @@
>> wctx = repo[None]
>> forgot = []
>> s = repo.status(match=match, clean=True)
>> - forget = sorted(s[0] + s[1] + s[3] + s[6])
>> + forget = sorted(s.modified + s.added + s.deleted + s.clean)
>> if explicitonly:
>> forget = [f for f in forget if match.exact(f)]
>> @@ -2187,8 +2187,8 @@
>> if len(old.parents()) > 1:
>> # ctx.files() isn't reliable for merges, so fall back to
>> the
>> # slower repo.status() method
>> - files = set([fn for st in repo.status(base, old)[:3]
>> - for fn in st])
>> + status = repo.status(base, old)
>> + files = set([fn for st in status.committable() for fn in
>> st])
>> else:
>> files = set(old.files())
>> @@ -2511,19 +2511,19 @@
>> unknown=True, ignored=True,
>> clean=True)
>> else:
>> changes = repo.status(match=m)
>> - for kind in changes:
>> + for kind in changes.all():
>> for abs in kind:
>> names[abs] = m.rel(abs), m.exact(abs)
>> m = scmutil.matchfiles(repo, names)
>> - modified = set(changes[0])
>> - added = set(changes[1])
>> - removed = set(changes[2])
>> - _deleted = set(changes[3])
>> - unknown = set(changes[4])
>> - unknown.update(changes[5])
>> - clean = set(changes[6])
>> + modified = set(changes.modified)
>> + added = set(changes.added)
>> + removed = set(changes.removed)
>> + _deleted = set(changes.deleted)
>
>
> Instead of refactoring an unused line, it would be nice to just remove it in
> an earlier changeset.
>
>> + unknown = set(changes.unknown)
>> + unknown.update(changes.ignored)
>> + clean = set(changes.clean)
>> # split between files known in target manifest and the others
>> smf = set(mf)
>> @@ -2544,9 +2544,9 @@
>> modified, added, removed = set(), set(), set()
>> else:
>> changes = repo.status(node1=parent, match=m)
>> - dsmodified = set(changes[0])
>> - dsadded = set(changes[1])
>> - dsremoved = set(changes[2])
>> + dsmodified = set(changes.modified)
>> + dsadded = set(changes.added)
>> + dsremoved = set(changes.removed)
>> # only take into account for removes between wc and target
>> clean |= dsremoved - removed
>> diff --git a/mercurial/commands.py b/mercurial/commands.py
>> --- a/mercurial/commands.py
>> +++ b/mercurial/commands.py
>> @@ -1491,9 +1491,9 @@
>> if not node:
>> stat = repo.status(match=scmutil.match(repo[None], pats,
>> opts))
>> - if stat[3]:
>> + if stat.deleted:
>> ui.status(_("nothing changed (%d missing files, see "
>> - "'hg status')\n") % len(stat[3]))
>> + "'hg status')\n") % len(stat.deleted))
>> else:
>> ui.status(_("nothing changed\n"))
>> return 1
>> @@ -3951,7 +3951,7 @@
>> parents = ctx.parents()
>> changed = ""
>> if default or id or num:
>> - if (util.any(repo.status())
>> + if (util.any(repo.status().all())
>> or util.any(ctx.sub(s).dirty() for s in
>> ctx.substate)):
>> changed = '+'
>> if default or id:
>> @@ -5149,7 +5149,6 @@
>> m = scmutil.match(repo[None], pats, opts)
>> s = repo.status(match=m, clean=True)
>> - modified, added, deleted, clean = s[0], s[1], s[3], s[6]
>> # warn about failure to delete explicit files/dirs
>> wctx = repo[None]
>> @@ -5165,19 +5164,19 @@
>> ret = 1
>> if force:
>> - list = modified + deleted + clean + added
>> + list = s.modified + s.deleted + s.clean + s.added
>> elif after:
>> - list = deleted
>> - for f in modified + added + clean:
>> + list = s.deleted
>> + for f in s.modified + s.added + s.clean:
>> ui.warn(_('not removing %s: file still exists\n') %
>> m.rel(f))
>> ret = 1
>> else:
>> - list = deleted + clean
>> - for f in modified:
>> + list = s.deleted + s.clean
>> + for f in s.modified:
>> ui.warn(_('not removing %s: file is modified (use -f'
>> ' to force removal)\n') % m.rel(f))
>> ret = 1
>> - for f in added:
>> + for f in s.added:
>> ui.warn(_('not removing %s: file has been marked for add'
>> ' (use forget to undo)\n') % m.rel(f))
>> ret = 1
>> @@ -5190,7 +5189,7 @@
>> try:
>> if not after:
>> for f in list:
>> - if f in added:
>> + if f in s.added:
>> continue # we never unlink added files on remove
>> util.unlinkpath(repo.wjoin(f), ignoremissing=True)
>> repo[None].forget(list)
>> @@ -5406,7 +5405,7 @@
>> hint = _("uncommitted merge, use --all to discard all
>> changes,"
>> " or 'hg update -C .' to abort the merge")
>> raise util.Abort(msg, hint=hint)
>> - dirty = util.any(repo.status())
>> + dirty = util.any(repo.status().all())
>> node = ctx.node()
>> if node != parent:
>> if dirty:
>> @@ -5711,7 +5710,14 @@
>> stat = repo.status(node1, node2, scmutil.match(repo[node2], pats,
>> opts),
>> 'ignored' in show, 'clean' in show, 'unknown' in
>> show,
>> opts.get('subrepos'))
>> - changestates = zip(states, 'MAR!?IC', stat)
>> + changestates = zip(states, 'MAR!?IC',
>> + [stat.modified,
>> + stat.added,
>> + stat.removed,
>> + stat.deleted,
>> + stat.unknown,
>> + stat.ignored,
>> + stat.clean])
>> if (opts.get('all') or opts.get('copies')) and not
>> opts.get('no_status'):
>> copy = copies.pathcopies(repo[node1], repo[node2])
>> @@ -5793,7 +5799,7 @@
>> ui.write(' ' + m, label='log.bookmark')
>> ui.write('\n', label='log.bookmark')
>> - st = list(repo.status(unknown=True))[:5]
>> + st = list(repo.status(unknown=True).all())[:5]
>> c = repo.dirstate.copies()
>> copied, renamed = [], []
>> diff --git a/mercurial/context.py b/mercurial/context.py
>> --- a/mercurial/context.py
>> +++ b/mercurial/context.py
>> @@ -14,6 +14,7 @@
>> import repoview
>> import fileset
>> import revlog
>> +from mercurial.dirstate import status
>> propertycache = util.propertycache
>> @@ -120,7 +121,7 @@
>> mf2 = self._manifestmatches(match, s)
>> modified, added, clean = [], [], []
>> - deleted, unknown, ignored = s[3], s[4], s[5]
>> + deleted, unknown, ignored = s.deleted, s.unknown, s.ignored
>> withflags = mf1.withflags() | mf2.withflags()
>> for fn, mf2node in mf2.iteritems():
>> if fn in mf1:
>> @@ -140,7 +141,8 @@
>> unknown = [fn for fn in unknown if fn not in mf1]
>> ignored = [fn for fn in ignored if fn not in mf1]
>> - return [modified, added, removed, deleted, unknown, ignored,
>> clean]
>> + return status(modified, added, removed, deleted, unknown,
>> ignored,
>> + clean)
>> @propertycache
>> def substate(self):
>> @@ -310,7 +312,7 @@
>> reversed = True
>> ctx1, ctx2 = ctx2, ctx1
>> - r = [[], [], [], [], [], [], []]
>> + r = status([], [], [], [], [], [], [])
>
>
> Shouldn't "empty list" be the default for status.__ini__?
>
>> match = ctx2._matchstatus(ctx1, r, match, listignored,
>> listclean,
>> listunknown)
>> r = ctx2._prestatus(ctx1, r, match, listignored, listclean,
>> listunknown)
>> @@ -321,7 +323,7 @@
>> if reversed:
>> # reverse added and removed
>> - r[1], r[2] = r[2], r[1]
>> + r.added, r.removed = r.removed, r.added
>> if listsubrepos:
>> for subpath, sub in scmutil.itersubrepos(ctx1, ctx2):
>> @@ -331,17 +333,16 @@
>> s = sub.status(rev2, match=submatch,
>> ignored=listignored,
>> clean=listclean, unknown=listunknown,
>> listsubrepos=True)
>> - for rfiles, sfiles in zip(r, s):
>> + for rfiles, sfiles in zip(r.all(), s.all()):
>> rfiles.extend("%s/%s" % (subpath, f) for f in
>> sfiles)
>> except error.LookupError:
>> self._repo.ui.status(_("skipping missing "
>> "subrepository: %s\n") %
>> subpath)
>> - for l in r:
>> + for l in r.all():
>> l.sort()
>> - # we return a tuple to signify that this list isn't changing
>> - return tuple(r)
>> + return r
>> def makememctx(repo, parents, text, user, date, branch, files,
>> store,
>> @@ -1050,8 +1051,7 @@
>> copied = self._repo.dirstate.copies()
>> ff = self._flagfunc
>> - modified, added, removed, deleted = self._status[:4]
>> - for i, l in (("a", added), ("m", modified)):
>> + for i, l in (("a", self._status.added), ("m",
>> self._status.modified)):
>> for f in l:
>> orig = copied.get(f, f)
>> man[f] = getman(orig).get(orig, nullid) + i
>> @@ -1060,7 +1060,7 @@
>> except OSError:
>> pass
>> - for f in deleted + removed:
>> + for f in self._status.deleted + self._status.removed:
>> if f in man:
>> del man[f]
>> @@ -1088,22 +1088,23 @@
>> def description(self):
>> return self._text
>> def files(self):
>> - return sorted(self._status[0] + self._status[1] +
>> self._status[2])
>> + return sorted(self._status.modified + self._status.added + \
>> + self._status.removed)
>> def modified(self):
>> - return self._status[0]
>> + return self._status.modified
>> def added(self):
>> - return self._status[1]
>> + return self._status.added
>> def removed(self):
>> - return self._status[2]
>> + return self._status.removed
>> def deleted(self):
>> - return self._status[3]
>> + return self._status.deleted
>> def unknown(self):
>> - return self._status[4]
>> + return self._status.unknown
>> def ignored(self):
>> - return self._status[5]
>> + return self._status.ignored
>> def clean(self):
>> - return self._status[6]
>> + return self._status.clean
>> def branch(self):
>> return encoding.tolocal(self._extra['branch'])
>> def closesbranch(self):
>> @@ -1378,11 +1379,10 @@
>> need to build a manifest and return what matches.
>> """
>> mf = self._repo['.']._manifestmatches(match, s)
>> - modified, added, removed = s[0:3]
>> - for f in modified + added:
>> + for f in s.modified + s.added:
>> mf[f] = None
>> mf.set(f, self.flags(f))
>> - for f in removed:
>> + for f in s.removed:
>> if f in mf:
>> del mf[f]
>> return mf
>> @@ -1405,8 +1405,8 @@
>> accidentally ended up with the entire contents of the file they
>> are
>> susposed to be linking to.
>> """
>> - s[0] = self._filtersuspectsymlink(s[0])
>> - self._status = s[:]
>> + s.modified = self._filtersuspectsymlink(s.modified)
>> + self._status = s
>> return s
>> def _dirstatestatus(self, match=None, ignored=False, clean=False,
>> @@ -1417,20 +1417,19 @@
>> subrepos = []
>> if '.hgsub' in self:
>> subrepos = sorted(self.substate)
>> - s = self._repo.dirstate.status(match, subrepos, listignored,
>> - listclean, listunknown)
>> - cmp, modified, added, removed, deleted, unknown, ignored, clean =
>> s
>> + (s, lookup) = self._repo.dirstate.status(match, subrepos,
>> listignored,
>> + listclean, listunknown)
>> # check for any possibly clean files
>> - if cmp:
>> - modified2, fixup = self._checklookup(cmp)
>> - modified += modified2
>> + if lookup:
>> + modified2, fixup = self._checklookup(lookup)
>> + s.modified += modified2
>> - # update dirstate for files that are actually clean
>> + # update status for files that are actually clean
>> if fixup and listclean:
>> - clean += fixup
>> + s.clean += fixup
>> - return [modified, added, removed, deleted, unknown, ignored,
>> clean]
>> + return s
>> def _buildstatus(self, other, s, match, listignored, listclean,
>> listunknown):
>> @@ -1478,10 +1477,8 @@
>> s = super(workingctx, self).status(other, match, listignored,
>> listclean,
>> listunknown, listsubrepos)
>> # calling 'super' subtly reveresed the contexts, so we flip the
>> results
>> - # (s[1] is 'added' and s[2] is 'removed')
>> - s = list(s)
>> - s[1], s[2] = s[2], s[1]
>> - return tuple(s)
>> + s.added, s.removed = s.removed, s.added
>> + return s
>> class committablefilectx(basefilectx):
>> """A committablefilectx provides common functionality for a file
>> context
>> @@ -1612,7 +1609,7 @@
>> p1, p2 = parents
>> self._parents = [changectx(self._repo, p) for p in (p1, p2)]
>> files = sorted(set(files))
>> - self._status = [files, [], [], [], []]
>> + self._status = status(files, [], [], [], [], [], [])
>> self._filectxfn = filectxfn
>> self.substate = {}
>> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
>> --- a/mercurial/dirstate.py
>> +++ b/mercurial/dirstate.py
>> @@ -26,6 +26,40 @@
>> def join(self, obj, fname):
>> return obj._join(fname)
>> +class status(object):
>
>
> Please describe the intention with this class.
>
>> + def __init__(self, modified, added, removed, deleted, unknown,
>> + ignored, clean):
>> + self.modified = modified
>> + self.added = added
>> + self.removed = removed
>> + self.deleted = deleted
>> + self.unknown = unknown
>> + self.ignored = ignored
>> + self.clean = clean
>
>
> http://mercurial.selenic.com/wiki/CodingStyle#Classes ... but I agree it is
> nicer than the 7-tuples.
Heh, I guess I should have read that first. I guess "recovering Java
programmer" describes me well, but I'm not sure I thought three times.
I did, however, ask on the IRC channel and heard no complaints (and no
encouragement either). I know Augie is for it.
> It is also very similar to a named tuple. Could we use that instead ... or
> inherit from it? (That would require some kludge as long as we support
> Python < 2.6.)
After having read the style guide, I know that extending from it is
not an option :-) I'll let others discuss and decide, since I know
nothing about Python. The ~8 lines of constructor doesn't seem that
bad to me.
>> +
>> + def all(self):
>> + return (self.modified,
>> + self.added,
>> + self.removed,
>> + self.deleted,
>> + self.unknown,
>> + self.ignored,
>> + self.clean)
>
>
> The name 'all' is not completely obvious. If that is the best possible name,
> it deserves a docstring.
>
> But I think a more Pythonic way (whatever that is) would be to define
> __iter__ to make it possible to say s = tuple(st)
>
>> + def changes(self):
>> + return (self.modified,
>> + self.added,
>> + self.removed,
>> + self.deleted)
>
>
> It seems like this (and committable) are convenience functions that could be
> added later.
Agreed. Will do.
> It do not return the changes but the files that have changed. I suggest
> calling it "changed".
>
>> + def haschanges(self):
>> + return util.any(self.changes())
>
>
> Comes for free with __iter__.
>
>> + def committable(self):
>> + return (self.modified,
>> + self.added,
>> + self.removed)
>
>
> Actually, this might be more like what I would call "changed".
>
> It is hard to come up with good names (although docstrings would help).
> Putting them in separate patches might make it more obvious if they are
> worth it.
>
>> @@ -900,8 +934,8 @@
>> elif state == 'r':
>> radd(fn)
>> - return (lookup, modified, added, removed, deleted, unknown,
>> ignored,
>> - clean)
>> + return (status(modified, added, removed, deleted, unknown,
>> + ignored, clean), lookup)
>> def matches(self, match):
>> '''
>> diff --git a/mercurial/fileset.py b/mercurial/fileset.py
>> --- a/mercurial/fileset.py
>> +++ b/mercurial/fileset.py
>> @@ -124,7 +124,7 @@
>> """
>> # i18n: "modified" is a keyword
>> getargs(x, 0, 0, _("modified takes no arguments"))
>> - s = mctx.status()[0]
>> + s = mctx.status().modified
>> return [f for f in mctx.subset if f in s]
>> def added(mctx, x):
>> @@ -133,7 +133,7 @@
>> """
>> # i18n: "added" is a keyword
>> getargs(x, 0, 0, _("added takes no arguments"))
>> - s = mctx.status()[1]
>> + s = mctx.status().added
>> return [f for f in mctx.subset if f in s]
>> def removed(mctx, x):
>> @@ -142,7 +142,7 @@
>> """
>> # i18n: "removed" is a keyword
>> getargs(x, 0, 0, _("removed takes no arguments"))
>> - s = mctx.status()[2]
>> + s = mctx.status().removed
>> return [f for f in mctx.subset if f in s]
>> def deleted(mctx, x):
>> @@ -151,7 +151,7 @@
>> """
>> # i18n: "deleted" is a keyword
>> getargs(x, 0, 0, _("deleted takes no arguments"))
>> - s = mctx.status()[3]
>> + s = mctx.status().deleted
>> return [f for f in mctx.subset if f in s]
>> def unknown(mctx, x):
>> @@ -161,7 +161,7 @@
>> """
>> # i18n: "unknown" is a keyword
>> getargs(x, 0, 0, _("unknown takes no arguments"))
>> - s = mctx.status()[4]
>> + s = mctx.status().unknown
>> return [f for f in mctx.subset if f in s]
>> def ignored(mctx, x):
>> @@ -171,7 +171,7 @@
>> """
>> # i18n: "ignored" is a keyword
>> getargs(x, 0, 0, _("ignored takes no arguments"))
>> - s = mctx.status()[5]
>> + s = mctx.status().ignored
>> return [f for f in mctx.subset if f in s]
>> def clean(mctx, x):
>> @@ -180,7 +180,7 @@
>> """
>> # i18n: "clean" is a keyword
>> getargs(x, 0, 0, _("clean takes no arguments"))
>> - s = mctx.status()[6]
>> + s = mctx.status().clean
>> return [f for f in mctx.subset if f in s]
>> def func(mctx, a, b):
>> @@ -448,8 +448,8 @@
>> return [f for f in files if f in self.subset]
>> def existing(self):
>> if self._status is not None:
>> - removed = set(self._status[3])
>> - unknown = set(self._status[4] + self._status[5])
>> + removed = set(self._status.deleted)
>> + unknown = set(self._status.unknown + self._status.ignored)
>> else:
>> removed = set()
>> unknown = set()
>> @@ -495,7 +495,7 @@
>> status = r.status(ctx.p1(), ctx,
>> unknown=unknown, ignored=ignored, clean=True)
>> subset = []
>> - for c in status:
>> + for c in status.all():
>> subset.extend(c)
>> else:
>> status = None
>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>> --- a/mercurial/localrepo.py
>> +++ b/mercurial/localrepo.py
>> @@ -574,7 +574,7 @@
>> date: date tuple to use if committing'''
>> if not local:
>> - for x in self.status()[:4]:
>> + for x in self.status().changes():
>> if '.hgtags' in x:
>> raise util.Abort(_('working copy of .hgtags is
>> changed '
>> '(please commit .hgtags
>> manually)'))
>> @@ -1229,9 +1229,10 @@
>> raise util.Abort(_('cannot partially commit a merge '
>> '(do not specify files or
>> patterns)'))
>> - changes = self.status(match=match, clean=force)
>> + status = self.status(match=match, clean=force)
>> if force:
>> - changes[0].extend(changes[6]) # mq may commit unchanged
>> files
>> + # mq may commit unchanged files
>> + status.modified.extend(status.clean)
>> # check subrepos
>> subs = []
>> @@ -1240,7 +1241,7 @@
>> # only manage subrepos and .hgsubstate if .hgsub is present
>> if '.hgsub' in wctx:
>> # we'll decide whether to track this ourselves, thanks
>> - for c in changes[:3]:
>> + for c in [status.modified, status.added, status.removed]:
>
>
> A place where commitable could be used? Except I don't think it is that bad
> to have it explicit ;-)
>
>> if '.hgsubstate' in c:
>> c.remove('.hgsubstate')
>> @@ -1278,23 +1279,24 @@
>> '.hgsub' in (wctx.modified() + wctx.added())):
>> raise util.Abort(
>> _("can't commit subrepos without .hgsub"))
>> - changes[0].insert(0, '.hgsubstate')
>> + status.modified.insert(0, '.hgsubstate')
>> - elif '.hgsub' in changes[2]:
>> + elif '.hgsub' in status.removed:
>> # clean up .hgsubstate when .hgsub is removed
>> if ('.hgsubstate' in wctx and
>> - '.hgsubstate' not in changes[0] + changes[1] +
>> changes[2]):
>> - changes[2].insert(0, '.hgsubstate')
>> + '.hgsubstate' not in (status.modified + status.added
>> +
>> + status.removed)):
>> + status.removed.insert(0, '.hgsubstate')
>> # make sure all explicit patterns are matched
>> if not force and match.files():
>> - matched = set(changes[0] + changes[1] + changes[2])
>> + matched = set(status.modified + status.added +
>> status.removed)
>> for f in match.files():
>> f = self.dirstate.normalize(f)
>> if f == '.' or f in matched or f in wctx.substate:
>> continue
>> - if f in changes[3]: # missing
>> + if f in status.deleted:
>> fail(f, _('file not found!'))
>> if f in vdirs: # visited directory
>> d = f + '/'
>> @@ -1306,7 +1308,7 @@
>> elif f not in self.dirstate:
>> fail(f, _("file not tracked!"))
>> - cctx = context.workingctx(self, text, user, date, extra,
>> changes)
>> + cctx = context.workingctx(self, text, user, date, extra,
>> status)
>> if (not force and not extra.get("close") and not merge
>> and not cctx.files()
>> @@ -1317,7 +1319,7 @@
>> raise util.Abort(_("cannot commit merge with missing
>> files"))
>> ms = mergemod.mergestate(self)
>> - for f in changes[0]:
>> + for f in status.modified:
>> if f in ms and ms[f] == 'u':
>> raise util.Abort(_("unresolved merge conflicts "
>> "(see hg help resolve)"))
>> diff --git a/mercurial/patch.py b/mercurial/patch.py
>> --- a/mercurial/patch.py
>> +++ b/mercurial/patch.py
>> @@ -1619,8 +1619,8 @@
>> ctx2 = repo[node2]
>> if not changes:
>> - changes = repo.status(ctx1, ctx2, match=match)
>> - modified, added, removed = changes[:3]
>> + changes = repo.status(ctx1, ctx2, match=match).committable()
>> + modified, added, removed = changes[0], changes[1], changes[2]
>
>
> That seems convoluted.
>
>> if not modified and not added and not removed:
>> return []
>> diff --git a/mercurial/revset.py b/mercurial/revset.py
>> --- a/mercurial/revset.py
>> +++ b/mercurial/revset.py
>> @@ -316,7 +316,7 @@
>> """
>> # i18n: "adds" is a keyword
>> pat = getstring(x, _("adds requires a pattern"))
>> - return checkstatus(repo, subset, pat, 1)
>> + return checkstatus(repo, subset, pat, 'added')
>
>
> Using indices was bad, but member names seems worse. Could it be done in a
> more elegant way?
>
>> def ancestor(repo, subset, x):
>> """``ancestor(*changeset)``
>> @@ -544,7 +544,7 @@
>> break
>> else:
>> return False
>> - files = repo.status(c.p1().node(), c.node())[field]
>> + files = repo.status(c.p1().node(), c.node()).__dict__[field]
>
>
> field -> fieldname ?
>
>> if fname is not None:
>> if fname in files:
>> return True
>> @@ -1112,7 +1112,7 @@
>> """
>> # i18n: "modifies" is a keyword
>> pat = getstring(x, _("modifies requires a pattern"))
>> - return checkstatus(repo, subset, pat, 0)
>> + return checkstatus(repo, subset, pat, 'modified')
>> def node_(repo, subset, x):
>> """``id(string)``
>> @@ -1336,7 +1336,7 @@
>> """
>> # i18n: "removes" is a keyword
>> pat = getstring(x, _("removes requires a pattern"))
>> - return checkstatus(repo, subset, pat, 2)
>> + return checkstatus(repo, subset, pat, 'removed')
>> def rev(repo, subset, x):
>> """``rev(number)``
>> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
>> --- a/mercurial/subrepo.py
>> +++ b/mercurial/subrepo.py
>> @@ -12,6 +12,7 @@
>> import config, util, node, error, cmdutil, bookmarks, match as matchmod
>> import phases
>> import pathutil
>> +from mercurial.dirstate import status as statusmod
>
>
> status is not a module - calling it statusmod is confusing. But as before,
> just import dirstate and use dirstate.status .
Oh, that's what 'mod' means. I just thought it meant 'modified' and
was used to avoid conflicts. Will fix.
>> hg = None
>> propertycache = util.propertycache
>> @@ -447,7 +448,7 @@
>> return 1
>> def status(self, rev2, **opts):
>> - return [], [], [], [], [], [], []
>> + return statusmod([], [], [], [], [], [], [])
>> def diff(self, ui, diffopts, node2, match, prefix, **opts):
>> pass
>> @@ -649,7 +650,7 @@
>> except error.RepoLookupError, inst:
>> self._repo.ui.warn(_('warning: error "%s" in subrepository
>> "%s"\n')
>> % (inst, subrelpath(self)))
>> - return [], [], [], [], [], [], []
>> + return statusmod([], [], [], [], [], [], [])
>> @annotatesubrepoerror
>> def diff(self, ui, diffopts, node2, match, prefix, **opts):
>> @@ -1564,7 +1565,7 @@
>> rev1 = self._state[1]
>> if self._gitmissing() or not rev1:
>> # if the repo is missing, return no results
>> - return [], [], [], [], [], [], []
>> + return statusmod([], [], [], [], [], [], [])
>> modified, added, removed = [], [], []
>> self._gitupdatestat()
>> if rev2:
>> @@ -1585,7 +1586,8 @@
>> removed.append(f)
>> deleted = unknown = ignored = clean = []
>> - return modified, added, removed, deleted, unknown, ignored, clean
>> + return statusmod(modified, added, removed, deleted, unknown,
>> ignored,
More information about the Mercurial-devel
mailing list