| Submitter | Sean Farley |
|---|---|
| Date | May 28, 2014, 5:48 p.m. |
| Message ID | <ebae0401cb6b85442061.1401299307@laptop.local> |
| Download | mbox | patch |
| Permalink | /patch/4893/ |
| State | Accepted |
| Commit | 0a8e7f81e8ae199d75241755534038f8201ae12c |
| Headers | show |
Comments
On 05/28/2014 10:48 AM, Sean Farley wrote: > # HG changeset patch > # User Sean Farley <sean.michael.farley@gmail.com> > # Date 1401228288 18000 > # Tue May 27 17:04:48 2014 -0500 > # Node ID ebae0401cb6b854420619962316005e814faad78 > # Parent 83bbfb23cb24b473286d528ddccbb333329f7f29 > context: explicitly return a tuple > > In the refactoring of removing localrepo.status, 2edb8648c500, we accidentally > changed the return type from a tuple to a list. Philosophically, this is > incorrect so we explicitly return a tuple again. > > diff --git a/mercurial/context.py b/mercurial/context.py > --- a/mercurial/context.py > +++ b/mercurial/context.py > @@ -324,11 +324,13 @@ class basectx(object): > self._repo.ui.status(_("skipping missing " > "subrepository: %s\n") % subpath) > > for l in r: > l.sort() > - return r > + > + # we return a tuple to signify that this list isn't changing > + return tuple(r) > > > def makememctx(repo, parents, text, user, date, branch, files, store, > editor=None): > def getfilectx(repo, memctx, path): > @@ -1444,11 +1446,11 @@ class workingctx(committablectx): > # 'memctx'? > 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[1], s[2] = s[2], s[1] > + s = s[2], s[1], s[3], s[4] Isn't this going to have awful silent consequences when 5 < len(s) or when someone write code under alcohol influence and silently drop the [0] item?
Pierre-Yves David <pierre-yves.david@ens-lyon.org> writes: > On 05/28/2014 10:48 AM, Sean Farley wrote: >> # HG changeset patch >> # User Sean Farley <sean.michael.farley@gmail.com> >> # Date 1401228288 18000 >> # Tue May 27 17:04:48 2014 -0500 >> # Node ID ebae0401cb6b854420619962316005e814faad78 >> # Parent 83bbfb23cb24b473286d528ddccbb333329f7f29 >> context: explicitly return a tuple >> >> In the refactoring of removing localrepo.status, 2edb8648c500, we accidentally >> changed the return type from a tuple to a list. Philosophically, this is >> incorrect so we explicitly return a tuple again. >> >> diff --git a/mercurial/context.py b/mercurial/context.py >> --- a/mercurial/context.py >> +++ b/mercurial/context.py >> @@ -324,11 +324,13 @@ class basectx(object): >> self._repo.ui.status(_("skipping missing " >> "subrepository: %s\n") % subpath) >> >> for l in r: >> l.sort() >> - return r >> + >> + # we return a tuple to signify that this list isn't changing >> + return tuple(r) >> >> >> def makememctx(repo, parents, text, user, date, branch, files, store, >> editor=None): >> def getfilectx(repo, memctx, path): >> @@ -1444,11 +1446,11 @@ class workingctx(committablectx): >> # 'memctx'? >> 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[1], s[2] = s[2], s[1] >> + s = s[2], s[1], s[3], s[4] > > Isn't this going to have awful silent consequences when 5 < len(s) or > when someone write code under alcohol influence and silently drop the > [0] item? Ugh, this was a bad overlook on my part. I'll fix it up.
Patch
diff --git a/mercurial/context.py b/mercurial/context.py --- a/mercurial/context.py +++ b/mercurial/context.py @@ -324,11 +324,13 @@ class basectx(object): self._repo.ui.status(_("skipping missing " "subrepository: %s\n") % subpath) for l in r: l.sort() - return r + + # we return a tuple to signify that this list isn't changing + return tuple(r) def makememctx(repo, parents, text, user, date, branch, files, store, editor=None): def getfilectx(repo, memctx, path): @@ -1444,11 +1446,11 @@ class workingctx(committablectx): # 'memctx'? 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[1], s[2] = s[2], s[1] + s = s[2], s[1], s[3], s[4] return s class committablefilectx(basefilectx): """A committablefilectx provides common functionality for a file context that wants the ability to commit, e.g. workingfilectx or memfilectx."""