Patchwork [17,of,24] churn: sort users with same churn by name

login
register
mail settings
Submitter Mads Kiilerich
Date Dec. 16, 2012, 10:34 p.m.
Message ID <bf6de588da8525999fe4.1355697252@localhost6.localdomain6>
Download mbox | patch
Permalink /patch/141/
State Accepted
Commit 2150e70c0ee10676751c0da4da9ebb426a6944ac
Headers show

Comments

Mads Kiilerich - Dec. 16, 2012, 10:34 p.m.
# HG changeset patch
# User Mads Kiilerich <mads at kiilerich.com>
# Date 1355276294 -3600
# Node ID bf6de588da8525999fe4ed9d9e813eb6ce5ea9b5
# Parent  998b984fd3463b81c38370e003602f0b16a887d5
churn: sort users with same churn by name

This makes the output order well-defined and improves code readability.
Bryan O'Sullivan - Dec. 17, 2012, 9:02 p.m.
On Sun, Dec 16, 2012 at 2:34 PM, Mads Kiilerich <mads at kiilerich.com> wrote:

> churn: sort users with same churn by name
>

I like the intent of this change.


> This makes the output order well-defined and improves code readability.
>
> -    sortkey = ((not opts.get('sort')) and (lambda x: -sum(x[1])) or None)
>

Improves? Hardly. That is impenetrable.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20121217/85652a99/attachment.html>
Mads Kiilerich - Dec. 18, 2012, 1:34 a.m.
Bryan O'Sullivan wrote, On 12/17/2012 10:02 PM:
> On Sun, Dec 16, 2012 at 2:34 PM, Mads Kiilerich <mads at kiilerich.com 
> <mailto:mads at kiilerich.com>> wrote:
>
>     churn: sort users with same churn by name
>
>
> I like the intent of this change.
>
>     This makes the output order well-defined and improves code
>     readability.
>
>     -    sortkey = ((not opts.get('sort')) and (lambda x: -sum(x[1]))
>     or None)
>
>
> Improves? Hardly. That is impenetrable.

Please clarify the intent of your comment. I agree that the removed line 
is non-obvious, but I don't get why you comment on the line I remove and 
don't consider the more explicit implementation and improvement.

It could be ironic, or it could be an opinion that is a consequence of 
too much experience functional programming ...

/Mads

> -    sortkey = ((not opts.get('sort')) and (lambda x: -sum(x[1])) or None)
> -    rate.sort(key=sortkey)
> +    if opts.get('sort'):
> +        rate.sort()
> +    else:
> +        rate.sort(key=lambda x: (-sum(x[1]), x))
Bryan O'Sullivan - Dec. 18, 2012, 6:44 p.m.
On Mon, Dec 17, 2012 at 5:34 PM, Mads Kiilerich <mads at kiilerich.com> wrote:

> Improves? Hardly. That is impenetrable.
>>
>
> Please clarify the intent of your comment.


You know what happened? I looked at the wrong hunk! Sorry about that.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20121218/fed45f43/attachment.html>

Patch

diff --git a/hgext/churn.py b/hgext/churn.py
--- a/hgext/churn.py
+++ b/hgext/churn.py
@@ -144,8 +144,10 @@ 
     if not rate:
         return
 
-    sortkey = ((not opts.get('sort')) and (lambda x: -sum(x[1])) or None)
-    rate.sort(key=sortkey)
+    if opts.get('sort'):
+        rate.sort()
+    else:
+        rate.sort(key=lambda x: (-sum(x[1]), x))
 
     # Be careful not to have a zero maxcount (issue833)
     maxcount = float(max(sum(v) for k, v in rate)) or 1.0
diff --git a/tests/test-churn.t b/tests/test-churn.t
--- a/tests/test-churn.t
+++ b/tests/test-churn.t
@@ -37,16 +37,16 @@ 
 churn all
 
   $ hg churn
+  user1      3 ***************************************************************
   user3      3 ***************************************************************
-  user1      3 ***************************************************************
   user2      2 ******************************************
 
 churn excluding one dir
 
   $ hg churn -X e
   user3      3 ***************************************************************
+  user1      2 ******************************************
   user2      2 ******************************************
-  user1      2 ******************************************
 
 churn up to rev 2
 
@@ -68,16 +68,16 @@ 
   $ mv ../aliases .hgchurn
   $ hg churn
   skipping malformed alias: not-an-alias
+  alias1      3 **************************************************************
   alias3      3 **************************************************************
-  alias1      3 **************************************************************
   user2       2 *****************************************
   $ rm .hgchurn
 
 churn with column specifier
 
   $ COLUMNS=40 hg churn
+  user1      3 ***********************
   user3      3 ***********************
-  user1      3 ***********************
   user2      2 ***************
 
 churn by hour
@@ -155,8 +155,8 @@ 
   $ hg churn -c
   user1            4 *********************************************************
   user3            3 *******************************************
+  user2            2 *****************************
   user4 at x.com      2 *****************************
-  user2            2 *****************************
   with space       1 **************
 
   $ cd ..