Submitter | Matt Mackall |
---|---|
Date | July 14, 2014, 11:17 p.m. |
Message ID | <1405379846.20966.195.camel@calx> |
Download | mbox | patch |
Permalink | /patch/5165/ |
State | Not Applicable |
Headers | show |
Comments
Thanks Matt, Your inputs are very helpful to me. Will come up with better contributions to the hg code :) Cheers, Prabhu On Tue, Jul 15, 2014 at 4:47 AM, Matt Mackall <mpm@selenic.com> wrote: > On Mon, 2014-07-14 at 21:46 +0530, Prabhu GS wrote: > > Hi Matt, > > > > I have fixed the style and used list comprehensions in the above patch. > But > > about the logic, I could not think of another approach. Any pointers > would > > help me fix it in a better way. > > It's not so much a matter of the logic but of the strategy. As I > mentioned before, we recently changed revsets to do "lazy evaluation": > > https://en.wikipedia.org/wiki/Lazy_evaluation > > That's what the minus operator here should be doing. So what needs > fixing is the minus operator. Explicitly looping over everything and > checking for membership is the opposite of lazy evaluation. Here's the > patch I came up with.. > > (Also notice my much smaller test reusing parts of the existing test as > described in > http://mercurial.selenic.com/wiki/WritingTests#No_more_new_test_scripts.21 > ) > > # HG changeset patch > # User Matt Mackall <mpm@selenic.com> > # Date 1405378531 18000 > # Mon Jul 14 17:55:31 2014 -0500 > # Node ID dd716807fd23d48677efd46bad8c759e3da2511e > # Parent e353fac7db2633c59d29a5059a08830a498aea49 > revset: maintain ordering when subtracting from a baseset (issue4289) > > diff -r e353fac7db26 -r dd716807fd23 mercurial/revset.py > --- a/mercurial/revset.py Tue Jul 15 00:59:09 2014 +0900 > +++ b/mercurial/revset.py Mon Jul 14 17:55:31 2014 -0500 > @@ -2243,11 +2243,7 @@ > """Returns a new object with the substraction of the two > collections. > > This is part of the mandatory API for smartset.""" > - if isinstance(other, baseset): > - s = other.set() > - else: > - s = set(other) > - return baseset(self.set() - s) > + return self.filter(lambda x: x not in other) > > def __and__(self, other): > """Returns a new object with the intersection of the two > collections. > diff -r e353fac7db26 -r dd716807fd23 tests/test-revset.t > --- a/tests/test-revset.t Tue Jul 15 00:59:09 2014 +0900 > +++ b/tests/test-revset.t Mon Jul 14 17:55:31 2014 -0500 > @@ -1008,6 +1008,11 @@ > $ log 'min(1 or 2) and not 1' > $ log 'last(1 or 2, 1) and not 2' > > +issue4289 - ordering of built-ins > + $ hg log -M -q -r 3:2 > + 3:8528aa5637f2 > + 2:5ed5505e9f1c > + > test revsets started with 40-chars hash (issue3669) > > $ ISSUE3669_TIP=`hg tip --template '{node}'` > > -- > Mathematics is the supreme nostalgia of our time. > > >
Patch
diff -r e353fac7db26 -r dd716807fd23 mercurial/revset.py --- a/mercurial/revset.py Tue Jul 15 00:59:09 2014 +0900 +++ b/mercurial/revset.py Mon Jul 14 17:55:31 2014 -0500 @@ -2243,11 +2243,7 @@ """Returns a new object with the substraction of the two collections. This is part of the mandatory API for smartset.""" - if isinstance(other, baseset): - s = other.set() - else: - s = set(other) - return baseset(self.set() - s) + return self.filter(lambda x: x not in other) def __and__(self, other): """Returns a new object with the intersection of the two collections. diff -r e353fac7db26 -r dd716807fd23 tests/test-revset.t --- a/tests/test-revset.t Tue Jul 15 00:59:09 2014 +0900 +++ b/tests/test-revset.t Mon Jul 14 17:55:31 2014 -0500 @@ -1008,6 +1008,11 @@ $ log 'min(1 or 2) and not 1' $ log 'last(1 or 2, 1) and not 2' +issue4289 - ordering of built-ins + $ hg log -M -q -r 3:2 + 3:8528aa5637f2 + 2:5ed5505e9f1c + test revsets started with 40-chars hash (issue3669) $ ISSUE3669_TIP=`hg tip --template '{node}'`