| Submitter | Pierre-Yves David |
|---|---|
| Date | June 22, 2015, 9:57 p.m. |
| Message ID | <5436ea241ec1c12294d7.1435010250@marginatus.alto.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/9753/ |
| State | Accepted |
| Headers | show |
Comments
On Mon, Jun 22, 2015 at 02:57:30PM -0700, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1435006081 25200 > # Mon Jun 22 13:48:01 2015 -0700 > # Node ID 5436ea241ec1c12294d7868de6c679d10d333060 > # Parent 7fdd1782fc4ee9da87d8af13e806dc9055db2c38 > revset: rework 'filteredset.last' queued, thanks > > As 'isascending' and 'isdescending' are method, not attribute. This led 'last()' > to misbehave on some non-ascending filtered set. > > diff --git a/mercurial/revset.py b/mercurial/revset.py > --- a/mercurial/revset.py > +++ b/mercurial/revset.py > @@ -3022,20 +3022,22 @@ class filteredset(abstractsmartset): > return x > return None > > def last(self): > it = None > - if self._subset.isascending: > + if self.isascending(): > it = self.fastdesc > - elif self._subset.isdescending: > - it = self.fastdesc > - if it is None: > - # slowly consume everything. This needs improvement > - it = lambda: reversed(list(self)) > - for x in it(): > + elif self.isdescending(): > + it = self.fastasc > + if it is not None: > + for x in it(): > + return x > + else: > + for x in self: > + pass > return x > - return None > + return None #empty case > > def __repr__(self): > return '<%s %r>' % (type(self).__name__, self._subset) > > # this function will be removed, or merged to addset or orset, when > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > https://selenic.com/mailman/listinfo/mercurial-devel
On Mon, 22 Jun 2015 14:57:30 -0700, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@fb.com> > # Date 1435006081 25200 > # Mon Jun 22 13:48:01 2015 -0700 > # Node ID 5436ea241ec1c12294d7868de6c679d10d333060 > # Parent 7fdd1782fc4ee9da87d8af13e806dc9055db2c38 > revset: rework 'filteredset.last' > > As 'isascending' and 'isdescending' are method, not attribute. This led 'last()' > to misbehave on some non-ascending filtered set. > > diff --git a/mercurial/revset.py b/mercurial/revset.py > --- a/mercurial/revset.py > +++ b/mercurial/revset.py > @@ -3022,20 +3022,22 @@ class filteredset(abstractsmartset): > return x > return None > > def last(self): > it = None > - if self._subset.isascending: > + if self.isascending(): > it = self.fastdesc > - elif self._subset.isdescending: > - it = self.fastdesc > - if it is None: > - # slowly consume everything. This needs improvement > - it = lambda: reversed(list(self)) > - for x in it(): > + elif self.isdescending(): > + it = self.fastasc > + if it is not None: > + for x in it(): > + return x > + else: > + for x in self: > + pass > return x It will fail if self is empty. >>> revset.filteredset(revset.generatorset([])).last() ... UnboundLocalError: local variable 'x' referenced before assignment
On Tue, Jun 23, 2015 at 10:37 AM, Yuya Nishihara <yuya@tcha.org> wrote: > On Mon, 22 Jun 2015 14:57:30 -0700, Pierre-Yves David wrote: >> # HG changeset patch >> # User Pierre-Yves David <pierre-yves.david@fb.com> >> # Date 1435006081 25200 >> # Mon Jun 22 13:48:01 2015 -0700 >> # Node ID 5436ea241ec1c12294d7868de6c679d10d333060 >> # Parent 7fdd1782fc4ee9da87d8af13e806dc9055db2c38 >> revset: rework 'filteredset.last' >> >> As 'isascending' and 'isdescending' are method, not attribute. This led 'last()' >> to misbehave on some non-ascending filtered set. >> >> diff --git a/mercurial/revset.py b/mercurial/revset.py >> --- a/mercurial/revset.py >> +++ b/mercurial/revset.py >> @@ -3022,20 +3022,22 @@ class filteredset(abstractsmartset): >> return x > > It will fail if self is empty. > >>>> revset.filteredset(revset.generatorset([])).last() > ... > UnboundLocalError: local variable 'x' referenced before assignment Nice catch. Should I drop this patch, or just await a followup? > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > https://selenic.com/mailman/listinfo/mercurial-devel
On 06/23/2015 12:55 PM, Augie Fackler wrote: > On Tue, Jun 23, 2015 at 10:37 AM, Yuya Nishihara <yuya@tcha.org> wrote: >> On Mon, 22 Jun 2015 14:57:30 -0700, Pierre-Yves David wrote: >>> # HG changeset patch >>> # User Pierre-Yves David <pierre-yves.david@fb.com> >>> # Date 1435006081 25200 >>> # Mon Jun 22 13:48:01 2015 -0700 >>> # Node ID 5436ea241ec1c12294d7868de6c679d10d333060 >>> # Parent 7fdd1782fc4ee9da87d8af13e806dc9055db2c38 >>> revset: rework 'filteredset.last' >>> >>> As 'isascending' and 'isdescending' are method, not attribute. This led 'last()' >>> to misbehave on some non-ascending filtered set. >>> >>> diff --git a/mercurial/revset.py b/mercurial/revset.py >>> --- a/mercurial/revset.py >>> +++ b/mercurial/revset.py >>> @@ -3022,20 +3022,22 @@ class filteredset(abstractsmartset): > >>> return x >> >> It will fail if self is empty. >> >>>>> revset.filteredset(revset.generatorset([])).last() >> ... >> UnboundLocalError: local variable 'x' referenced before assignment > > Nice catch. Should I drop this patch, or just await a followup? Just before the loop add x = None (please)
On Tue, Jun 23, 2015 at 4:06 PM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote: >> Nice catch. Should I drop this patch, or just await a followup? > > > Just before the loop add > > x = None > > (please) Please send me a diff, or amend the patch that's on the clowncopter?
On 06/23/2015 01:07 PM, Augie Fackler wrote: > On Tue, Jun 23, 2015 at 4:06 PM, Pierre-Yves David > <pierre-yves.david@ens-lyon.org> wrote: >>> Nice catch. Should I drop this patch, or just await a followup? >> >> >> Just before the loop add >> >> x = None >> >> (please) > > > Please send me a diff, or amend the patch that's on the clowncopter? I've amended it and will push back once the test are done running.
Patch
diff --git a/mercurial/revset.py b/mercurial/revset.py --- a/mercurial/revset.py +++ b/mercurial/revset.py @@ -3022,20 +3022,22 @@ class filteredset(abstractsmartset): return x return None def last(self): it = None - if self._subset.isascending: + if self.isascending(): it = self.fastdesc - elif self._subset.isdescending: - it = self.fastdesc - if it is None: - # slowly consume everything. This needs improvement - it = lambda: reversed(list(self)) - for x in it(): + elif self.isdescending(): + it = self.fastasc + if it is not None: + for x in it(): + return x + else: + for x in self: + pass return x - return None + return None #empty case def __repr__(self): return '<%s %r>' % (type(self).__name__, self._subset) # this function will be removed, or merged to addset or orset, when