From 301214dd39102675004efd4bc5f98e62233bbd92 Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Thu, 17 Mar 2016 14:48:39 -0700 Subject: [PATCH 1/5] Fix List splice method to be spec compatible If the second argument is omitted, then all objects after the provided index should be removed. This is the same behavior as Array.prototype.splice. --- src/js_list.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/js_list.cpp b/src/js_list.cpp index 4a8ca1b0..7acbdadf 100644 --- a/src/js_list.cpp +++ b/src/js_list.cpp @@ -174,14 +174,20 @@ JSValueRef ListSplice(JSContextRef ctx, JSObjectRef function, JSObjectRef thisOb List *list = RJSGetInternal(thisObject); size_t size = list->size(); - RJSValidateArgumentCountIsAtLeast(argumentCount, 2); + RJSValidateArgumentCountIsAtLeast(argumentCount, 1); long index = std::min(RJSValidatedValueToNumber(ctx, arguments[0]), size); if (index < 0) { index = std::max(size + index, 0); } - long remove = std::max(RJSValidatedValueToNumber(ctx, arguments[1]), 0); - remove = std::min(remove, size - index); + long remove; + if (argumentCount < 2) { + remove = size - index; + } + else { + remove = std::max(RJSValidatedValueToNumber(ctx, arguments[1]), 0); + remove = std::min(remove, size - index); + } std::vector removedObjects(remove); for (size_t i = 0; i < remove; i++) { From 933326c41014129d4507e5abf03e356a6280e993 Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Thu, 17 Mar 2016 14:49:12 -0700 Subject: [PATCH 2/5] Add List splice method to API docs It was mysteriously missing! --- docs/list.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/list.js b/docs/list.js index 79ee73cc..94d1fecb 100644 --- a/docs/list.js +++ b/docs/list.js @@ -16,7 +16,6 @@ // //////////////////////////////////////////////////////////////////////////// - /** * Instances of this class will be returned when accessing object properties whose type is `"list"` * (see {@linkplain Realm~ObjectSchemaProperty ObjectSchemaProperty}). @@ -89,6 +88,19 @@ class List { */ push(...object) {} + /** + * Changes the contents of the list by removing objects and/or inserting new objects. + * @see {@linkcode https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice Array.prototype.splice} + * @param {number} index - The start index. If greater than the length of the list, + * the start index will be set to the length instead. If negative, then the start index + * will be counted from the end of the list (e.g. `list.length - index`). + * @param {number} [count=length - start] - The number of objects to remove from the list. + * @param {...Realm.Object} [object] - Objects to insert into the list starting at `index`. + * @returns {Realm.Object[]} containing the objects that were removed from the list. The + * array is empty if no objects were removed. + */ + splice(index, count, ...object) {} + /** * Add one or more objects to the _beginning_ of the list. * @param {...Realm.Object} object - Each object’s type must match From 9ba477762ab4458c571e074db5f91614d274f46f Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Thu, 17 Mar 2016 14:53:42 -0700 Subject: [PATCH 3/5] Update splice tests to test added behavior --- tests/js/list-tests.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/js/list-tests.js b/tests/js/list-tests.js index 34d79028..0a93c913 100644 --- a/tests/js/list-tests.js +++ b/tests/js/list-tests.js @@ -344,6 +344,14 @@ module.exports = BaseTest.extend({ TestCase.assertEqual(removed.length, 0); TestCase.assertEqual(array.length, 1); + removed = array.splice(1); + TestCase.assertEqual(removed.length, 0); + TestCase.assertEqual(array.length, 1); + + removed = array.splice(0); + TestCase.assertEqual(removed.length, 1); + TestCase.assertEqual(array.length, 0); + TestCase.assertThrows(function() { array.splice('cat', 1); }); From f8de504ac6345d7145b66dfada67f471c3ff0503 Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Thu, 17 Mar 2016 14:56:07 -0700 Subject: [PATCH 4/5] Update CHANGELOG with splice method fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 273f4c29..e507ec05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ * Fix for using Chrome debug mode from a device * Automatically forward port 8082 for Android * Fix broken iterator methods on Android +* Fix for List splice method not accepting a single argument * Don't download or unpack core libraries unnecessarily From aa5529e059ed47b257524194e7db48c41d26e028 Mon Sep 17 00:00:00 2001 From: Scott Kyle Date: Thu, 17 Mar 2016 15:12:18 -0700 Subject: [PATCH 5/5] Update List splice doc to be more clear about count --- docs/list.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/list.js b/docs/list.js index 94d1fecb..e3b5b873 100644 --- a/docs/list.js +++ b/docs/list.js @@ -94,7 +94,8 @@ class List { * @param {number} index - The start index. If greater than the length of the list, * the start index will be set to the length instead. If negative, then the start index * will be counted from the end of the list (e.g. `list.length - index`). - * @param {number} [count=length - start] - The number of objects to remove from the list. + * @param {number} [count] - The number of objects to remove from the list. If not provided, + * then all objects from the start index through the end of the list will be removed. * @param {...Realm.Object} [object] - Objects to insert into the list starting at `index`. * @returns {Realm.Object[]} containing the objects that were removed from the list. The * array is empty if no objects were removed.