Patchwork dispatch: fix so that if the -M flag does not change the order of records in hg log (issue4289)

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)

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.
>
>
>