fixes #19: array (patch): fixes CommentArray::sort

test/array: more test cases

Author: kaelzhang

src/array.js

@@ -69,6 +69,17 @@ const move_comments = (
   }
 }
 
+const get_mapped = (map, key) => {
+  let mapped = key
+
+  while (mapped in map) {
+    mapped = map[mapped]
+  }
+
+  return mapped
+}
+
+
 class CommentArray extends Array {
   // - deleteCount + items.length
 
@@ -225,36 +236,49 @@ class CommentArray extends Array {
     )
 
     // For example,
-    // if we sort ['c', 'a', 'b', 'd'],
-    // then `result` will be [1, 2, 0, 3], and the array is ['a', 'b', 'c', 'd']
+    // if we sort ['b', 'd', 'c', 'a'],
+    // then `result` will be [3, 0, 2, 1], and the array is ['a', 'b', 'c', 'd']
+
+    // First, we swap index 0 (b) and index 3 (a), then the array comments are
+    // ['a.comments', 'd.comments', 'c.comments', 'b.comments']
+    // index 0 is finalized
+    // index 3 is actually mapped to original index 0, we present as 0 -> 3
 
-    // First, we swap index 0 and index 1, then the array comments are
-    // ['a.comments', 'c.comments', 'b.comments', 'd.comments']
+    // Then swap index 1 (d) and index 0 (-> 3, b)
+    // 1 (index) -> 0 (new index) -> 3 (real_index)
+    // ['d.comments', 'b.comments', 'c.comments', 'd.comments']
+    // index 1 is finalized
+    // index 3 is contains the item of original index 1
+    // - we present as 1 -> 3
+    // - it is ok that we don't remove mapping 0 -> 3
 
-    // Then swap index 1 and index 2
-    // ['a.comments', 'b.comments', 'c.comments', 'd.comments']
+    // Then index 2 should be skipped
 
-    // And we should not swap index 2 and index 0
+    // Then swap index 3 (d) and index 1 (-> 3, b), skipped
 
-    const sorted = new Set()
+    const map = Object.create(null)
 
-    result.forEach((new_index, original_index) => {
-      if (new_index === original_index) {
+    result.forEach((source_index, index) => {
+      if (source_index === index) {
         return
       }
 
-      if (sorted.has(new_index) && sorted.has(original_index)) {
+      const real_source_index = get_mapped(map, source_index)
+
+      if (real_source_index === index) {
         return
       }
 
-      sorted.add(new_index)
-      sorted.add(original_index)
+      // The item of index `index` gets the final value
+      // delete map[index]
+      map[index] = real_source_index
 
-      swap_comments(this, new_index, original_index)
+      swap_comments(this, index, real_source_index)
     })
   }
 }
 
+
 module.exports = {
   CommentArray
 }

test/array.test.js

@@ -224,9 +224,58 @@ const CASES = [
   // 1
   1
 ]`)
-  ]
+  ],
+  [
+    'sort, default lex sort',
+    `[
+  // before c
+  "c",
+  "a", // after a
+  "b" /* after b value */,
+  "d"
+]`,
+    array => {
+      array.sort()
+      return array
+    },
+    unshift(['a', 'b', 'c', 'd'], `[
+  "a", // after a
+  "b" /* after b value */,
+  // before c
+  "c",
+  "d"
+]`)
+  ],
+  [
+    'sort, with `compareFunction`',
+    `[
+  // before c
+  "c",
+  "a", // after a
+  "b" /* after b value */,
+  // before d
+  "d"
+]`,
+    array => {
+      array.sort(
+        (a, b) => a > b
+          ? - 1
+          : 1
+      )
+      return array
+    },
+    unshift(['d', 'c', 'b', 'a'], `[
+  // before d
+  "d",
+  // before c
+  "c",
+  "b" /* after b value */,
+  "a" // after a
+]`)
+  ],
 ]
 
+
 CASES.forEach(([d, a, run, e, s]) => {
   test(d, t => {
     const parsed = parse(a)