Patchwork [1,of,3,V2,STABLE] changelog: introduce count argument to revs() function

login
register
mail settings
Submitter Alexander Plavin
Date July 22, 2013, 9:54 a.m.
Message ID <82096c966790ec607efb.1374486865@debian-alexander.dolgopa>
Download mbox | patch
Permalink /patch/1943/
State Superseded
Headers show

Comments

Alexander Plavin - July 22, 2013, 9:54 a.m.
# HG changeset patch
# User Alexander Plavin <me@aplavin.ru>
# Date 1374321959 -14400
#      Sat Jul 20 16:05:59 2013 +0400
# Node ID 82096c966790ec607efb16ebff2cfa53c643b700
# Parent  b97ce93726dbcbfc439458e244a266953c22940e
changelog: introduce count argument to revs() function

This allows specifying count of revisions to yield, which is convenient when
some revs are filtered.
Brodie Rao - July 22, 2013, 11:47 p.m.
On Mon, Jul 22, 2013 at 2:54 AM, Alexander Plavin <me@aplavin.ru> wrote:
> # HG changeset patch
> # User Alexander Plavin <me@aplavin.ru>
> # Date 1374321959 -14400
> #      Sat Jul 20 16:05:59 2013 +0400
> # Node ID 82096c966790ec607efb16ebff2cfa53c643b700
> # Parent  b97ce93726dbcbfc439458e244a266953c22940e
> changelog: introduce count argument to revs() function
>
> This allows specifying count of revisions to yield, which is convenient when
> some revs are filtered.
>
> diff -r b97ce93726db -r 82096c966790 mercurial/changelog.py
> --- a/mercurial/changelog.py    Fri Jul 19 21:26:08 2013 +0400
> +++ b/mercurial/changelog.py    Sat Jul 20 16:05:59 2013 +0400
> @@ -147,11 +147,23 @@
>
>          return filterediter()
>
> -    def revs(self, start=0, stop=None):
> +    def revs(self, start=0, stop=None, count=None):
>          """filtered version of revlog.revs"""
> +        if count is not None:
> +            if count < 0:
> +                stop = 0
> +            elif count > 0:
> +                stop = len(self)
> +            else:
> +                stop = start
> +
> +        curcount = 0
>          for i in super(changelog, self).revs(start, stop):
>              if i not in self.filteredrevs:
>                  yield i
> +                curcount += 1
> +                if count is not None and curcount >= abs(count):
> +                    break

Instead of making this change inside changelog.revs(), couldn't you
use itertools.islice() where you need to limit the number of
revisions?

>      @util.propertycache
>      def nodemap(self):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Alexander Plavin - July 23, 2013, 9:08 a.m.
2013/7/23 Brodie Rao <brodie@sf.io>:
> On Mon, Jul 22, 2013 at 2:54 AM, Alexander Plavin <me@aplavin.ru> wrote:
>> # HG changeset patch
>> # User Alexander Plavin <me@aplavin.ru>
>> # Date 1374321959 -14400
>> #      Sat Jul 20 16:05:59 2013 +0400
>> # Node ID 82096c966790ec607efb16ebff2cfa53c643b700
>> # Parent  b97ce93726dbcbfc439458e244a266953c22940e
>> changelog: introduce count argument to revs() function
>>
>> This allows specifying count of revisions to yield, which is convenient when
>> some revs are filtered.
>>
>> diff -r b97ce93726db -r 82096c966790 mercurial/changelog.py
>> --- a/mercurial/changelog.py    Fri Jul 19 21:26:08 2013 +0400
>> +++ b/mercurial/changelog.py    Sat Jul 20 16:05:59 2013 +0400
>> @@ -147,11 +147,23 @@
>>
>>          return filterediter()
>>
>> -    def revs(self, start=0, stop=None):
>> +    def revs(self, start=0, stop=None, count=None):
>>          """filtered version of revlog.revs"""
>> +        if count is not None:
>> +            if count < 0:
>> +                stop = 0
>> +            elif count > 0:
>> +                stop = len(self)
>> +            else:
>> +                stop = start
>> +
>> +        curcount = 0
>>          for i in super(changelog, self).revs(start, stop):
>>              if i not in self.filteredrevs:
>>                  yield i
>> +                curcount += 1
>> +                if count is not None and curcount >= abs(count):
>> +                    break
>
> Instead of making this change inside changelog.revs(), couldn't you
> use itertools.islice() where you need to limit the number of
> revisions?

I thought about such variant, but this one (from this patch) seemed
more convenient to use in the calling place and more universal.
('revs(start, -count)' vs 'itertools.islice(revs(start, 0), count)').
If anyone else agrees that with islice it will be better, I will
change and resend.

>
>>      @util.propertycache
>>      def nodemap(self):
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
Alexander Plavin - July 23, 2013, 9:22 a.m.
2013/7/23 Alexander Plavin <me@aplavin.ru>:
> 2013/7/23 Brodie Rao <brodie@sf.io>:
>> On Mon, Jul 22, 2013 at 2:54 AM, Alexander Plavin <me@aplavin.ru> wrote:
>>> # HG changeset patch
>>> # User Alexander Plavin <me@aplavin.ru>
>>> # Date 1374321959 -14400
>>> #      Sat Jul 20 16:05:59 2013 +0400
>>> # Node ID 82096c966790ec607efb16ebff2cfa53c643b700
>>> # Parent  b97ce93726dbcbfc439458e244a266953c22940e
>>> changelog: introduce count argument to revs() function
>>>
>>> This allows specifying count of revisions to yield, which is convenient when
>>> some revs are filtered.
>>>
>>> diff -r b97ce93726db -r 82096c966790 mercurial/changelog.py
>>> --- a/mercurial/changelog.py    Fri Jul 19 21:26:08 2013 +0400
>>> +++ b/mercurial/changelog.py    Sat Jul 20 16:05:59 2013 +0400
>>> @@ -147,11 +147,23 @@
>>>
>>>          return filterediter()
>>>
>>> -    def revs(self, start=0, stop=None):
>>> +    def revs(self, start=0, stop=None, count=None):
>>>          """filtered version of revlog.revs"""
>>> +        if count is not None:
>>> +            if count < 0:
>>> +                stop = 0
>>> +            elif count > 0:
>>> +                stop = len(self)
>>> +            else:
>>> +                stop = start
>>> +
>>> +        curcount = 0
>>>          for i in super(changelog, self).revs(start, stop):
>>>              if i not in self.filteredrevs:
>>>                  yield i
>>> +                curcount += 1
>>> +                if count is not None and curcount >= abs(count):
>>> +                    break
>>
>> Instead of making this change inside changelog.revs(), couldn't you
>> use itertools.islice() where you need to limit the number of
>> revisions?
>
> I thought about such variant, but this one (from this patch) seemed
> more convenient to use in the calling place and more universal.
> ('revs(start, -count)' vs 'itertools.islice(revs(start, 0), count)').
> If anyone else agrees that with islice it will be better, I will
> change and resend.

Also, (forgot to add to the previous message) it relies on the
function returning an iterator, but not a list of requested revisions
and uses this knowledge (if it was returning list,
'itertools.islice(revs(start, 0), count)' would be very inefficient).

>
>>
>>>      @util.propertycache
>>>      def nodemap(self):
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel@selenic.com
>>> http://selenic.com/mailman/listinfo/mercurial-devel
Matt Mackall - July 23, 2013, 9:12 p.m.
On Tue, 2013-07-23 at 13:22 +0400, Alexander Plavin wrote:
> 2013/7/23 Alexander Plavin <me@aplavin.ru>:
> > 2013/7/23 Brodie Rao <brodie@sf.io>:
> >> On Mon, Jul 22, 2013 at 2:54 AM, Alexander Plavin <me@aplavin.ru> wrote:
> >>> # HG changeset patch
> >>> # User Alexander Plavin <me@aplavin.ru>
> >>> # Date 1374321959 -14400
> >>> #      Sat Jul 20 16:05:59 2013 +0400
> >>> # Node ID 82096c966790ec607efb16ebff2cfa53c643b700
> >>> # Parent  b97ce93726dbcbfc439458e244a266953c22940e
> >>> changelog: introduce count argument to revs() function
> >>>
> >>> This allows specifying count of revisions to yield, which is convenient when
> >>> some revs are filtered.
> >>>
> >>> diff -r b97ce93726db -r 82096c966790 mercurial/changelog.py
> >>> --- a/mercurial/changelog.py    Fri Jul 19 21:26:08 2013 +0400
> >>> +++ b/mercurial/changelog.py    Sat Jul 20 16:05:59 2013 +0400
> >>> @@ -147,11 +147,23 @@
> >>>
> >>>          return filterediter()
> >>>
> >>> -    def revs(self, start=0, stop=None):
> >>> +    def revs(self, start=0, stop=None, count=None):
> >>>          """filtered version of revlog.revs"""
> >>> +        if count is not None:
> >>> +            if count < 0:
> >>> +                stop = 0
> >>> +            elif count > 0:
> >>> +                stop = len(self)
> >>> +            else:
> >>> +                stop = start
> >>> +
> >>> +        curcount = 0
> >>>          for i in super(changelog, self).revs(start, stop):
> >>>              if i not in self.filteredrevs:
> >>>                  yield i
> >>> +                curcount += 1
> >>> +                if count is not None and curcount >= abs(count):
> >>> +                    break
> >>
> >> Instead of making this change inside changelog.revs(), couldn't you
> >> use itertools.islice() where you need to limit the number of
> >> revisions?
> >
> > I thought about such variant, but this one (from this patch) seemed
> > more convenient to use in the calling place and more universal.
> > ('revs(start, -count)' vs 'itertools.islice(revs(start, 0), count)').
> > If anyone else agrees that with islice it will be better, I will
> > change and resend.
> 
> Also, (forgot to add to the previous message) it relies on the
> function returning an iterator, but not a list of requested revisions
> and uses this knowledge (if it was returning list,
> 'itertools.islice(revs(start, 0), count)' would be very inefficient).

I've got bad news for you:

a) Trying to change changelog.revs just for hgweb is not terribly
desirable, especially on stable.
b) I have an irrational hatred of itertools

Just open-code it. Probably with changelog.hasnode.

Patch

diff -r b97ce93726db -r 82096c966790 mercurial/changelog.py
--- a/mercurial/changelog.py	Fri Jul 19 21:26:08 2013 +0400
+++ b/mercurial/changelog.py	Sat Jul 20 16:05:59 2013 +0400
@@ -147,11 +147,23 @@ 
 
         return filterediter()
 
-    def revs(self, start=0, stop=None):
+    def revs(self, start=0, stop=None, count=None):
         """filtered version of revlog.revs"""
+        if count is not None:
+            if count < 0:
+                stop = 0
+            elif count > 0:
+                stop = len(self)
+            else:
+                stop = start
+
+        curcount = 0
         for i in super(changelog, self).revs(start, stop):
             if i not in self.filteredrevs:
                 yield i
+                curcount += 1
+                if count is not None and curcount >= abs(count):
+                    break
 
     @util.propertycache
     def nodemap(self):