refactor: New Editor.positions based off PR#3644 (ianstormtaylor#4199)

Also fixes `Editor.positions` bug ianstormtaylor#3458 that was fixed in parallel in ianstormtaylor#4073,
but includes refactorings as discussed in ianstormtaylor#3644.

vs ianstormtaylor#3458
- Updated to include changes from later PRs (ianstormtaylor#3957)
- Does not add test cases (relies on those from ianstormtaylor#4073)
- Minor improvements on comments

Author: beorn

packages/slate/src/interfaces/editor.ts

@@ -1211,16 +1211,16 @@ export const Editor: EditorInterface = {
   },
 
   /**
-   * Iterate through all of the positions in the document where a `Point` can be
-   * placed.
+   * Return all the positions in `at` range where a `Point` can be placed.
    *
-   * By default it will move forward by individual offsets at a time,  but you
-   * can pass the `unit: 'character'` option to moved forward one character, word,
-   * or line at at time.
+   * By default, moves forward by individual offsets at a time, but
+   * the `unit` option can be used to to move by character, word, line, or block.
+   *
+   * The `reverse` option can be used to change iteration direction.
    *
    * Note: By default void nodes are treated as a single point and iteration
    * will not happen inside their content unless you pass in true for the
-   * voids option, then iteration will occur.
+   * `voids` option, then iteration will occur.
    */
 
   *positions(
@@ -1243,100 +1243,160 @@ export const Editor: EditorInterface = {
       return
     }
 
+    /**
+     * Algorithm notes:
+     *
+     * Each step `distance` is dynamic depending on the underlying text
+     * and the `unit` specified.  Each step, e.g., a line or word, may
+     * span multiple text nodes, so we iterate through the text both on
+     * two levels in step-sync:
+     *
+     * `leafText` stores the text on a text leaf level, and is advanced
+     * through using the counters `leafTextOffset` and `leafTextRemaining`.
+     *
+     * `blockText` stores the text on a block level, and is shortened
+     * by `distance` every time it is advanced.
+     *
+     * We only maintain a window of one blockText and one leafText because
+     * a block node always appears before all of its leaf nodes.
+     */
+
     const range = Editor.range(editor, at)
     const [start, end] = Range.edges(range)
     const first = reverse ? end : start
-    let string = ''
-    let available = 0
-    let offset = 0
-    let distance: number | null = null
     let isNewBlock = false
-
-    const advance = () => {
-      if (distance == null) {
-        if (unit === 'character') {
-          distance = getCharacterDistance(string)
-        } else if (unit === 'word') {
-          distance = getWordDistance(string)
-        } else if (unit === 'line' || unit === 'block') {
-          distance = string.length
-        } else {
-          distance = 1
-        }
-
-        string = string.slice(distance)
-      }
-
-      // Add or substract the offset.
-      offset = reverse ? offset - distance : offset + distance
-      // Subtract the distance traveled from the available text.
-      available = available - distance!
-      // If the available had room to spare, reset the distance so that it will
-      // advance again next time. Otherwise, set it to the overflow amount.
-      distance = available >= 0 ? null : 0 - available
-    }
-
+    let blockText = ''
+    let distance = 0 // Distance for leafText to catch up to blockText.
+    let leafTextRemaining = 0
+    let leafTextOffset = 0
+
+    // Iterate through all nodes in range, grabbing entire textual content
+    // of block nodes in blockText, and text nodes in leafText.
+    // Exploits the fact that nodes are sequenced in such a way that we first
+    // encounter the block node, then all of its text nodes, so when iterating
+    // through the blockText and leafText we just need to remember a window of
+    // one block node and leaf node, respectively.
     for (const [node, path] of Editor.nodes(editor, { at, reverse, voids })) {
+      /*
+       * ELEMENT NODE - Yield position(s) for voids, collect blockText for blocks
+       */
       if (Element.isElement(node)) {
         // Void nodes are a special case, so by default we will always
-        // yield their first point. If the voids option is set to true,
-        // then we will iterate over their content
+        // yield their first point. If the `voids` option is set to true,
+        // then we will iterate over their content.
         if (!voids && editor.isVoid(node)) {
           yield Editor.start(editor, path)
           continue
         }
 
-        if (editor.isInline(node)) {
-          continue
-        }
+        // Inline element nodes are ignored as they don't themselves
+        // contribute to `blockText` or `leafText` - their parent and
+        // children do.
+        if (editor.isInline(node)) continue
 
+        // Block element node - set `blockText` to its text content.
         if (Editor.hasInlines(editor, node)) {
+          // We always exhaust block nodes before encountering a new one:
+          //   console.assert(blockText === '',
+          //     `blockText='${blockText}' - `+
+          //     `not exhausted before new block node`, path)
+
+          // Ensure range considered is capped to `range`, in the
+          // start/end edge cases where block extends beyond range.
+          // Equivalent to this, but presumably more performant:
+          //   blockRange = Editor.range(editor, ...Editor.edges(editor, path))
+          //   blockRange = Range.intersection(range, blockRange) // intersect
+          //   blockText = Editor.string(editor, blockRange, { voids })
           const e = Path.isAncestor(path, end.path)
             ? end
             : Editor.end(editor, path)
           const s = Path.isAncestor(path, start.path)
             ? start
             : Editor.start(editor, path)
 
-          const text = Editor.string(editor, { anchor: s, focus: e }, { voids })
-          string = reverse ? reverseText(text) : text
+          blockText = Editor.string(editor, { anchor: s, focus: e }, { voids })
+          blockText = reverse ? reverseText(blockText) : blockText
           isNewBlock = true
         }
       }
 
+      /*
+       * TEXT LEAF NODE - Iterate through text content, yielding
+       * positions every `distance` offset according to `unit`.
+       */
       if (Text.isText(node)) {
         const isFirst = Path.equals(path, first.path)
-        available = node.text.length
-        offset = reverse ? available : 0
 
+        // Proof that we always exhaust text nodes before encountering a new one:
+        //   console.assert(leafTextRemaining <= 0,
+        //     `leafTextRemaining=${leafTextRemaining} - `+
+        //     `not exhausted before new leaf text node`, path)
+
+        // Reset `leafText` counters for new text node.
         if (isFirst) {
-          available = reverse ? first.offset : available - first.offset
-          offset = first.offset
+          leafTextRemaining = reverse
+            ? first.offset
+            : node.text.length - first.offset
+          leafTextOffset = first.offset // Works for reverse too.
+        } else {
+          leafTextRemaining = node.text.length
+          leafTextOffset = reverse ? leafTextRemaining : 0
         }
 
+        // Yield position at the start of node (potentially).
         if (isFirst || isNewBlock || unit === 'offset') {
-          yield { path, offset }
+          yield { path, offset: leafTextOffset }
+          isNewBlock = false
         }
 
+        // Yield positions every (dynamically calculated) `distance` offset.
         while (true) {
-          // If there's no more string and there is no more characters to skip, continue to the next block.
-          if (string === '' && distance === null) {
-            break
-          } else {
-            advance()
+          // If `leafText` has caught up with `blockText` (distance=0),
+          // and if blockText is exhausted, break to get another block node,
+          // otherwise advance blockText forward by the new `distance`.
+          if (distance === 0) {
+            if (blockText === '') break
+            distance = calcDistance(blockText, unit)
+            blockText = blockText.slice(distance)
           }
 
-          // If the available space hasn't overflow, we have another point to
-          // yield in the current text node.
-          if (available >= 0) {
-            yield { path, offset }
-          } else {
+          // Advance `leafText` by the current `distance`.
+          leafTextOffset = reverse
+            ? leafTextOffset - distance
+            : leafTextOffset + distance
+          leafTextRemaining = leafTextRemaining - distance
+
+          // If `leafText` is exhausted, break to get a new leaf node
+          // and set distance to the overflow amount, so we'll (maybe)
+          // catch up to blockText in the next leaf text node.
+          if (leafTextRemaining < 0) {
+            distance = -leafTextRemaining
             break
           }
-        }
 
-        isNewBlock = false
+          // Successfully walked `distance` offsets through `leafText`
+          // to catch up with `blockText`, so we can reset `distance`
+          // and yield this position in this node.
+          distance = 0
+          yield { path, offset: leafTextOffset }
+        }
+      }
+    }
+    // Proof that upon completion, we've exahusted both leaf and block text:
+    //   console.assert(leafTextRemaining <= 0, "leafText wasn't exhausted")
+    //   console.assert(blockText === '', "blockText wasn't exhausted")
+
+    // Helper:
+    // Return the distance in offsets for a step of size `unit` on given string.
+    function calcDistance(text: string, unit: string) {
+      if (unit === 'character') {
+        return getCharacterDistance(text)
+      } else if (unit === 'word') {
+        return getWordDistance(text)
+      } else if (unit === 'line' || unit === 'block') {
+        return text.length
       }
+      return 1
     }
   },