Repository: zauberzeug/nicegui
Pull request #4676 (2025-04-28 16:48:28)

Simplify change set processing for `ui.codemirror`
In preparation for PR #4578, this PR significantly simplifies the processing of change sets in `ui.codemirror`.

An optimization, the inner while loop, has been removed. I think user input, which is processed in `_event_args_to_value`, usually consists of one section only and we don't need the more complex algorithm to process consecutive changes at once.

evnchn: > I think user input, which is processed in _event_args_to_value, usually consists of one section only and we don't need the more complex algorithm to process consecutive changes at once.

CodeMirror is a black-box which I don't comprehend, but I think it may still be beneficial if we process multiple changes, _in the rare case that if we have multiple changes_ we can process them all. 

Still, I understand your desire for clean code. Perhaps we can get rid of the nested while loop, by defining the inner while loop's functionality into a function (which we will shortly modify for the UTF-16 compatibility)

Personally don't want to "take away" functionality in a PR unless we are absolutely sure, which we aren't, I'm afraid. 
falkoschindler: @evnchn I don't think this PR "takes away" functionality. It just removes a potential performance improvement, which - as far as I can tell - is irrelevant for our application.

Here is an explanation from Cursor (referring to the old implementation), which shows how simply these change sets actually are:

> The `_iter_changes` function processes a `ChangeSet` which describes modifications to a document. The `changeset.sections` list encodes these modifications in pairs of integers: `(length_in_old_doc, length_in_new_doc_or_-1)`.
> 
> - A pair like `(length, -1)` means a section of `length` characters remains unchanged.
> - A pair like `(old_length, new_length)` (where `new_length >= 0`) signifies a change (insertion, deletion, or replacement).
> 
> The outer `while` loop iterates through these section pairs.
> 
> The inner `while True` loop specifically handles sequences of *consecutive* changes. It accumulates the changes from adjacent modified sections until it encounters:
> 1.  An unchanged section (`changeset.sections[i + 1] < 0`).
> 2.  The end of the `changeset.sections` list.
> 3.  The `individual` flag is set (though in the `apply` method call, `individual` is `False`).
> 
> By grouping consecutive changes, the inner loop calculates the total span of the modification in the original document (`posA` to `endA`) and the resulting span in the new document (`posB` to `endB`), along with concatenating all the inserted text (`text`) for that entire block.
> 
> After the inner loop breaks, the collected change (represented by `posA`, `endA`, `posB`, `endB`, and `text`) is applied to the document in a single operation using the `func`. This approach avoids applying many small, adjacent changes individually, potentially improving efficiency.

My implementation captures the essence, without the need of an extra class, an iterator, function arguments, and multiple smaller functions calling each other. Hopefully it makes the algorithm more comprehensible. And the additional pytest should make sure that we don't introduce regressions in the future.

Patch:
diff --git a/nicegui/elements/codemirror.py b/nicegui/elements/codemirror.py
index f5f9326b..47c4f146 100644
--- a/nicegui/elements/codemirror.py
+++ b/nicegui/elements/codemirror.py
@@ -1,5 +1,6 @@
+from itertools import zip_longest
 from pathlib import Path
-from typing import Callable, List, Literal, Optional, get_args
+from typing import List, Literal, Optional, Tuple, cast, get_args
 
 from nicegui.elements.mixins.disableable_element import DisableableElement
 from nicegui.elements.mixins.value_element import ValueElement
@@ -332,80 +333,17 @@ class CodeMirror(ValueElement, DisableableElement, component='codemirror.js', de
 
     def _event_args_to_value(self, e: GenericEventArguments) -> str:
         """The event contains a change set which is applied to the current value."""
-        changeset = _ChangeSet(sections=e.args['sections'], inserted=e.args['inserted'])
-        new_value = changeset.apply(self.value)
-        return new_value
-
-
-# Below is a Python implementation of relevant parts of https://github.com/codemirror/state/blob/main/src/change.ts
-# to apply a ChangeSet to a text document.
-
-
-class _ChangeSet:
-    """A change set represents a group of modifications to a document."""
-
-    def __init__(self, sections: List[int], inserted: List[List[str]]) -> None:
-        # From https://github.com/codemirror/state/blob/main/src/change.ts#L21:
-        # Sections are encoded as pairs of integers. The first is the
-        # length in the current document, and the second is -1 for
-        # unaffected sections, and the length of the replacement content
-        # otherwise. So an insertion would be (0, n>0), a deletion (n>0,
-        # 0), and a replacement two positive numbers.
-        self.sections: List[int] = sections
-        self.inserted: List[str] = ['\n'.join(ins) for ins in inserted]
-
-    def length(self) -> int:
-        """Calculate the length of the document before the change."""
-        return sum(self.sections[::2])
-
-    def apply(self, doc: str) -> str:
-        """Apply the changes to a document, returning the modified document."""
-        if self.length() != len(doc):
-            raise ValueError('Cannot apply change set to a document with the wrong length')
-        return _iter_changes(self, doc, _replacement_func, individual=False)
-
-    def __str__(self) -> str:
-        return f'ChangeSet(sections={self.sections}, inserted={self.inserted})'
-
-
-def _iter_changes(
-    changeset: _ChangeSet, doc: str, func: Callable[[str, int, int, int, int, str], str], individual: bool
-) -> str:
-    inserted = changeset.inserted
-    posA, posB, i = 0, 0, 0
-
-    while i < len(changeset.sections):
-        len_ = changeset.sections[i]
-        i += 1
-        ins = changeset.sections[i]
-        i += 1
-
-        if ins < 0:
-            posA += len_
-            posB += len_
-        else:
-            endA, endB = posA, posB
-            text = ''
-            while True:
-                endA += len_
-                endB += ins
-                if ins and inserted:
-                    text = text + inserted[(i - 2) // 2]
-                if individual or i == len(changeset.sections) or changeset.sections[i + 1] < 0:
-                    break
-                len_ = changeset.sections[i]
-                i += 1
-                ins = changeset.sections[i]
-                i += 1
-            doc = func(doc, posA, endA, posB, endB, text)
-            posA, posB = endA, endB
-
+        return _apply_change_set(self.value, e.args['sections'], e.args['inserted'])
+
+
+def _apply_change_set(doc, sections: List[int], inserted: List[List[str]]) -> str:
+    # based on https://github.com/codemirror/state/blob/main/src/change.ts
+    assert sum(sections[::2]) == len(doc), 'Cannot apply change set to document due to length mismatch'
+    pos = 0
+    joined_inserts = ('\n'.join(ins) for ins in inserted)
+    for section in zip_longest(sections[::2], sections[1::2], joined_inserts, fillvalue=''):
+        old_len, new_len, ins = cast(Tuple[int, int, str], section)
+        if new_len >= 0:
+            doc = doc[:pos] + ins + doc[pos + old_len:]
+        pos += old_len
     return doc
-
-
-def _replace_range(doc: str, from_: int, to: int, new: str) -> str:
-    return doc[:from_] + new + doc[to:]
-
-
-def _replacement_func(doc: str, from_a: int, to_a: int, from_b: int, _to_b: int, text: str) -> str:
-    return _replace_range(doc, from_b, from_b + (to_a - from_a), text)
diff --git a/nicegui/static/nicegui.js b/nicegui/static/nicegui.js
index c7ed5db9..e65b3db0 100644
--- a/nicegui/static/nicegui.js
+++ b/nicegui/static/nicegui.js
@@ -35,7 +35,7 @@ function replaceUndefinedAttributes(element) {
 }
 
 function getElement(id) {
-  const _id = id instanceof HTMLElement ? id.id : id;
+  const _id = id instanceof Element ? id.id.slice(1) : id;
   return mounted_app.$refs["r" + _id];
 }
 


Test patch:
diff --git a/tests/test_codemirror.py b/tests/test_codemirror.py
index e73d533c..97d7ba8d 100644
--- a/tests/test_codemirror.py
+++ b/tests/test_codemirror.py
@@ -1,6 +1,7 @@
 from typing import Dict, List
 
 from nicegui import ui
+from nicegui.elements.codemirror import _apply_change_set
 from nicegui.testing import Screen
 
 
@@ -31,3 +32,13 @@ def test_supported_values(screen: Screen):
     screen.wait_for('Done')
     assert values['languages'] == values['supported_languages']
     assert values['themes'] == values['supported_themes']
+
+
+def test_change_set():
+    assert _apply_change_set('', [0, 1], [['A']]) == 'A'
+    assert _apply_change_set('', [0, 2], [['AB']]) == 'AB'
+    assert _apply_change_set('X', [1, 2], [['AB']]) == 'AB'
+    assert _apply_change_set('X', [1, -1], []) == 'X'
+    assert _apply_change_set('X', [1, -1, 0, 1], [[], ['Y']]) == 'XY'
+    assert _apply_change_set('Hello', [5, -1, 0, 8], [[], [', world!']]) == 'Hello, world!'
+    assert _apply_change_set('Hello, world!', [5, -1, 7, 0, 1, -1], []) == 'Hello!'