From d7a91117ac30d103c5e4451cc44af1d8442bcc99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thorbj=C3=B8rn=20Lindeijer?= Date: Sun, 15 Mar 2015 22:51:24 +0100 Subject: [PATCH] Reverse the argument order of List.insert The previous order, insert(element, index), was counter-intuitive. I'm not aware of any list API that uses this order. I've checked: * Ruby Array.insert(index, obj...) * JavaScript array.splice(start, deleteCount[, item1[, item2[, ...]]]) * C++ / QList::insert(int i, const T & value) * C++ / std::vector::insert * Lua table.insert (list, [pos,] value) * C# List.Insert(int index, T item) * Java Interface List.add(int index, E element) * Python list.insert(i, x) So it seemed to me more like an oversight in Wren. --- doc/site/core/list.markdown | 2 +- doc/site/lists.markdown | 10 +++++----- src/vm/wren_core.c | 8 ++++---- test/core/list/insert.wren | 20 ++++++++++---------- test/core/list/insert_index_not_int.wren | 2 +- test/core/list/insert_index_not_num.wren | 2 +- test/core/list/insert_index_too_large.wren | 2 +- test/core/list/insert_index_too_small.wren | 2 +- 8 files changed, 24 insertions(+), 24 deletions(-) diff --git a/doc/site/core/list.markdown b/doc/site/core/list.markdown index cef9e36e..4e08007e 100644 --- a/doc/site/core/list.markdown +++ b/doc/site/core/list.markdown @@ -17,7 +17,7 @@ Removes all items from the list. The number of items in the list. -### **insert**(item, index) +### **insert**(index, item) **TODO** diff --git a/doc/site/lists.markdown b/doc/site/lists.markdown index 36986ce4..1c6e475a 100644 --- a/doc/site/lists.markdown +++ b/doc/site/lists.markdown @@ -71,10 +71,10 @@ use `add` to append a single item to the end: You can insert a new element at a specific position using `insert`: :::dart - hirsute.insert("soul patch", 2) + hirsute.insert(2, "soul patch") -The first argument is the value to insert, and the second is the index to -insert it at. All elements following the inserted one will be pushed down to +The first argument is the index to insert at, and the second is the value to +insert. All elements following the inserted one will be pushed down to make room for it. It's valid to "insert" after the last element in the list, but only *right* @@ -83,9 +83,9 @@ back. Doing so counts back from the size of the list *after* it's grown by one: :::dart var letters = ["a", "b", "c"] - letters.insert("d", 3) // OK: inserts at end. + letters.insert(3, "d") // OK: inserts at end. IO.print(letters) // ["a", "b", "c", "d"] - letters.insert("e", -2) // Counts back from size after insert. + letters.insert(-2, "e") // Counts back from size after insert. IO.print(letters) // ["a", "b", "c", "e", "d"] ## Removing elements diff --git a/src/vm/wren_core.c b/src/vm/wren_core.c index 32b9e23b..a48c1edd 100644 --- a/src/vm/wren_core.c +++ b/src/vm/wren_core.c @@ -380,7 +380,7 @@ DEF_PRIMITIVE(class_supertype) // Object has no superclass. if (classObj->superclass == NULL) RETURN_NULL; - + RETURN_OBJ(classObj->superclass); } @@ -694,11 +694,11 @@ DEF_PRIMITIVE(list_insert) ObjList* list = AS_LIST(args[0]); // count + 1 here so you can "insert" at the very end. - int index = validateIndex(vm, args, list->count + 1, 2, "Index"); + int index = validateIndex(vm, args, list->count + 1, 1, "Index"); if (index == -1) return PRIM_ERROR; - wrenListInsert(vm, list, args[1], index); - RETURN_VAL(args[1]); + wrenListInsert(vm, list, args[2], index); + RETURN_VAL(args[2]); } DEF_PRIMITIVE(list_iterate) diff --git a/test/core/list/insert.wren b/test/core/list/insert.wren index 44097630..ed471a60 100644 --- a/test/core/list/insert.wren +++ b/test/core/list/insert.wren @@ -1,41 +1,41 @@ // Add to empty list. var a = [] -a.insert(1, 0) +a.insert(0, 1) IO.print(a) // expect: [1] // Normal indices. var b = [1, 2, 3] -b.insert(4, 0) +b.insert(0, 4) IO.print(b) // expect: [4, 1, 2, 3] var c = [1, 2, 3] -c.insert(4, 1) +c.insert(1, 4) IO.print(c) // expect: [1, 4, 2, 3] var d = [1, 2, 3] -d.insert(4, 2) +d.insert(2, 4) IO.print(d) // expect: [1, 2, 4, 3] var e = [1, 2, 3] -e.insert(4, 3) +e.insert(3, 4) IO.print(e) // expect: [1, 2, 3, 4] // Negative indices. var f = [1, 2, 3] -f.insert(4, -4) +f.insert(-4, 4) IO.print(f) // expect: [4, 1, 2, 3] var g = [1, 2, 3] -g.insert(4, -3) +g.insert(-3, 4) IO.print(g) // expect: [1, 4, 2, 3] var h = [1, 2, 3] -h.insert(4, -2) +h.insert(-2, 4) IO.print(h) // expect: [1, 2, 4, 3] var i = [1, 2, 3] -i.insert(4, -1) +i.insert(-1, 4) IO.print(i) // expect: [1, 2, 3, 4] // Returns.inserted value. -IO.print([1, 2].insert(3, 0)) // expect: 3 +IO.print([1, 2].insert(0, 3)) // expect: 3 diff --git a/test/core/list/insert_index_not_int.wren b/test/core/list/insert_index_not_int.wren index f382cc4e..18ddea6a 100644 --- a/test/core/list/insert_index_not_int.wren +++ b/test/core/list/insert_index_not_int.wren @@ -1,2 +1,2 @@ var a = [1, 2, 3] -a.insert("value", 1.5) // expect runtime error: Index must be an integer. +a.insert(1.5, "value") // expect runtime error: Index must be an integer. diff --git a/test/core/list/insert_index_not_num.wren b/test/core/list/insert_index_not_num.wren index d046d3a2..5f0de497 100644 --- a/test/core/list/insert_index_not_num.wren +++ b/test/core/list/insert_index_not_num.wren @@ -1,2 +1,2 @@ var a = [1, 2, 3] -a.insert("value", "2") // expect runtime error: Index must be a number. +a.insert("2", "value") // expect runtime error: Index must be a number. diff --git a/test/core/list/insert_index_too_large.wren b/test/core/list/insert_index_too_large.wren index a193f8ad..02830329 100644 --- a/test/core/list/insert_index_too_large.wren +++ b/test/core/list/insert_index_too_large.wren @@ -1,2 +1,2 @@ var a = [1, 2, 3] -a.insert("value", 4) // expect runtime error: Index out of bounds. +a.insert(4, "value") // expect runtime error: Index out of bounds. diff --git a/test/core/list/insert_index_too_small.wren b/test/core/list/insert_index_too_small.wren index 433d2f4d..7189f5dc 100644 --- a/test/core/list/insert_index_too_small.wren +++ b/test/core/list/insert_index_too_small.wren @@ -1,2 +1,2 @@ var a = [1, 2, 3] -a.insert("value", -5) // expect runtime error: Index out of bounds. +a.insert(-5, "value") // expect runtime error: Index out of bounds.