Issue 43 make sort(reverse=True) work

There were two possible approaches to making key=value parameters work and I took what I saw as the safe impact with the most limited impact. I also tried code in function.js that automatically added the co_varnames to all functions defined using Sk.builtin.func as follows: if ( code ) { funStr = code.toString(); parms= funStr.slice(funStr.indexOf('(')+1, funStr.indexOf(')')).match(/([^\s,]+)/g); if ( parms.length > 0 ) { this.func_code['co_varnames']=parms; } } This code worked for sort(reverse=True), but lots of code started to fail in the unit tests having to do with parameter passing and Python defined functions. It would be good in the above code to only trigger on native JS library functions.
Duncanr-glitch · Jan 26, 2013 · 45d2ad8 · 45d2ad8
1 parent 5d984a6
commit 45d2ad8
Show file tree

Hide file tree

Showing 6 changed files with 101 additions and 10 deletions.
diff --git a/src/list.js b/src/list.js
@@ -321,9 +321,12 @@ Sk.builtin.list.prototype['reverse'] = new Sk.builtin.func(function(self)
 Sk.builtin.list.prototype['sort'] = new Sk.builtin.func(function(self, cmp, key, reverse)
 {
     goog.asserts.assert(!key, "todo;");
-    goog.asserts.assert(!reverse, "todo;");
-    Sk.mergeSort(self.v, cmp);
+    Sk.mergeSort(self.v, cmp, key, reverse);
     return null;
 });
 
+// Make sure that key/value variations of lst.sort() work
+// See issue 45 on github as to possible alternate approaches to this and
+// why this was chosen - csev
+Sk.builtin.list.prototype['sort'].func_code['co_varnames']=['__self__','cmp', 'key', 'reverse'];
 goog.exportSymbol("Sk.builtin.list", Sk.builtin.list);
diff --git a/src/mergesort.js b/src/mergesort.js
@@ -17,14 +17,13 @@ Sk.mergeSort = function(arr, cmp, key, reverse)	//	Replaced by quicksort
 Sk.quickSort = function(arr, cmp, key, reverse)
 {
     goog.asserts.assert(!key, "todo;");
-    goog.asserts.assert(!reverse, "todo;");
 
     if (!cmp)
     {
         cmp = Sk.mergeSort.stdCmp;
     }
 
-    var partition = function(arr, begin, end, pivot)
+    var partition = function(arr, begin, end, pivot, reverse)
 	{
 		var tmp;
 		var piv=arr[pivot];
@@ -37,7 +36,12 @@ Sk.quickSort = function(arr, cmp, key, reverse)
 		var store=begin;
 		var ix;
 		for(ix=begin; ix<end-1; ++ix) {
-			if(Sk.misceval.callsim(cmp, arr[ix], piv) < 0) { // arr[ix]<=piv) {
+            if ( reverse ) {
+			  var cmpresult = Sk.misceval.callsim(cmp, piv, arr[ix]);
+            } else {
+			  var cmpresult = Sk.misceval.callsim(cmp, arr[ix], piv);
+            }
+            if( cmpresult < 0 ) {
 //				swap store, ix
 				tmp=arr[store];
 				arr[store]=arr[ix];
@@ -54,19 +58,19 @@ Sk.quickSort = function(arr, cmp, key, reverse)
 		return store;
 	}
 
-	var qsort = function(arr, begin, end)
+	var qsort = function(arr, begin, end, reverse)
 	{
 		if(end-1>begin) {
 			var pivot=begin+Math.floor(Math.random()*(end-begin));
 
-			pivot=partition(arr, begin, end, pivot);
+			pivot=partition(arr, begin, end, pivot, reverse);
 
-			qsort(arr, begin, pivot);
-			qsort(arr, pivot+1, end);
+			qsort(arr, begin, pivot, reverse);
+			qsort(arr, pivot+1, end, reverse);
 		}
 	}
 
-    qsort(arr, 0, arr.length);
+    qsort(arr, 0, arr.length, reverse);
     return null;
 };
 

diff --git a/test/run/t338.py b/test/run/t338.py
@@ -0,0 +1,8 @@
+l = [9,8,1,2,3,45,5]
+l.sort()
+print l
+print "---------------- DEFAULT"
+l.sort(reverse=True)
+print l
+print "---------------- REVERSE"
+
diff --git a/test/run/t338.py.real b/test/run/t338.py.real
@@ -0,0 +1,4 @@
+[1, 2, 3, 5, 8, 9, 45]
+---------------- DEFAULT
+[45, 9, 8, 5, 3, 2, 1]
+---------------- REVERSE
diff --git a/test/run/t338.py.symtab b/test/run/t338.py.symtab
@@ -0,0 +1,30 @@
+Sym_type: module
+Sym_name: top
+Sym_lineno: 0
+Sym_nested: False
+Sym_haschildren: False
+-- Identifiers --
+name: True
+  is_referenced: True
+  is_imported: False
+  is_parameter: False
+  is_global: True
+  is_declared_global: False
+  is_local: False
+  is_free: False
+  is_assigned: False
+  is_namespace: False
+  namespaces: [
+  ]
+name: l
+  is_referenced: True
+  is_imported: False
+  is_parameter: False
+  is_global: False
+  is_declared_global: False
+  is_local: True
+  is_free: False
+  is_assigned: True
+  is_namespace: False
+  namespaces: [
+  ]
diff --git a/test/run/t338.trans b/test/run/t338.trans
@@ -0,0 +1,42 @@
+Module(body=[Assign(targets=[Name(id='l',
+                                  ctx=Store())],
+                    value=List(elts=[Num(n=9),
+                                     Num(n=8),
+                                     Num(n=1),
+                                     Num(n=2),
+                                     Num(n=3),
+                                     Num(n=45),
+                                     Num(n=5)],
+                               ctx=Load())),
+             Expr(value=Call(func=Attribute(value=Name(id='l',
+                                                       ctx=Load()),
+                                            attr='sort',
+                                            ctx=Load()),
+                             args=[],
+                             keywords=[],
+                             starargs=None,
+                             kwargs=None)),
+             Print(dest=None,
+                   values=[Name(id='l',
+                                ctx=Load())],
+                   nl=True),
+             Print(dest=None,
+                   values=[Str(s='---------------- DEFAULT')],
+                   nl=True),
+             Expr(value=Call(func=Attribute(value=Name(id='l',
+                                                       ctx=Load()),
+                                            attr='sort',
+                                            ctx=Load()),
+                             args=[],
+                             keywords=[keyword(arg='reverse',
+                                               value=Name(id='True',
+                                                          ctx=Load()))],
+                             starargs=None,
+                             kwargs=None)),
+             Print(dest=None,
+                   values=[Name(id='l',
+                                ctx=Load())],
+                   nl=True),
+             Print(dest=None,
+                   values=[Str(s='---------------- REVERSE')],
+                   nl=True)])