fix undo/redo in BigMonospaceText should restore selection as well

src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigMonospaceText.kt

@@ -299,6 +299,15 @@ private fun CoreBigMonospaceText(
 //    log.v { "text = |${text.buildString()}|" }
 //    log.v { "transformedText = |${transformedText.buildString()}|" }
 
+    remember(text, viewState) {
+        text.undoMetadataSupplier = {
+            BigTextUndoMetadata(
+                cursor = viewState.cursorIndex,
+                selection = viewState.selection,
+            )
+        }
+    }
+
     fun fireOnLayout() {
         lineHeight = (textLayouter.charMeasurer as ComposeUnicodeCharMeasurer).getRowHeight()
         onTextLayout?.let { callback ->
@@ -455,6 +464,26 @@ private fun CoreBigMonospaceText(
         )
     }
 
+    fun scrollToCursor() {
+        val layoutResult = layoutResult ?: return
+
+        // scroll to cursor position if out of visible range
+        val visibleVerticalRange = scrollState.value .. scrollState.value + height
+        val row = transformedText.findRowIndexByPosition(viewState.transformedCursorIndex)
+        val rowVerticalRange = layoutResult.getTopOfRow(row).toInt() .. layoutResult.getBottomOfRow(row).toInt()
+        if (rowVerticalRange !in visibleVerticalRange) {
+            val scrollToPosition = if (rowVerticalRange.start < visibleVerticalRange.start) {
+                rowVerticalRange.start
+            } else {
+                // scroll to a position that includes the bottom of the row + a little space
+                minOf(layoutResult.bottom.toInt(), maxOf(0, rowVerticalRange.endInclusive + maxOf(2, (layoutResult.rowHeight * 0.5).toInt()) - height))
+            }
+            coroutineScope.launch {
+                scrollState.animateScrollTo(scrollToPosition)
+            }
+        }
+    }
+
     fun updateViewState() {
         viewState.lastVisibleRow = minOf(viewState.lastVisibleRow, transformedText.lastRowIndex)
         log.d { "lastVisibleRow = ${viewState.lastVisibleRow}, lastRowIndex = ${transformedText.lastRowIndex}" }
@@ -498,15 +527,16 @@ private fun CoreBigMonospaceText(
             val start = viewState.selection.start
             val endExclusive = viewState.selection.endInclusive + 1
             delete(start, endExclusive)
-            if (isSaveUndoSnapshot) {
-                text.recordCurrentChangeSequenceIntoUndoHistory()
-            }
 
             viewState.selection = EMPTY_SELECTION_RANGE // cannot use IntRange.EMPTY as `viewState.selection.start` is in use
             viewState.transformedSelection = EMPTY_SELECTION_RANGE
             viewState.cursorIndex = start
             viewState.updateTransformedCursorIndexByOriginal(transformedText)
             viewState.transformedSelectionStart = viewState.transformedCursorIndex
+
+            if (isSaveUndoSnapshot) {
+                text.recordCurrentChangeSequenceIntoUndoHistory()
+            }
         }
     }
 
@@ -524,9 +554,6 @@ private fun CoreBigMonospaceText(
         }
         val insertPos = viewState.cursorIndex
         insertAt(insertPos, textInput)
-        if (isSaveUndoSnapshot) {
-            text.recordCurrentChangeSequenceIntoUndoHistory()
-        }
         updateViewState()
         if (log.config.minSeverity <= Severity.Verbose) {
             (transformedText as BigTextImpl).printDebug("transformedText onType '${textInput.string().replace("\n", "\\n")}'")
@@ -536,6 +563,9 @@ private fun CoreBigMonospaceText(
         viewState.updateTransformedCursorIndexByOriginal(transformedText)
         viewState.transformedSelectionStart = viewState.transformedCursorIndex
         log.v { "set cursor pos 2 => ${viewState.cursorIndex} t ${viewState.transformedCursorIndex}" }
+        if (isSaveUndoSnapshot) {
+            text.recordCurrentChangeSequenceIntoUndoHistory()
+        }
     }
 
     fun onDelete(direction: TextFBDirection): Boolean {
@@ -552,20 +582,19 @@ private fun CoreBigMonospaceText(
                 if (cursor + 1 <= text.length) {
                     onValuePreChange(BigTextChangeEventType.Delete, cursor, cursor + 1)
                     text.delete(cursor, cursor + 1)
-                    text.recordCurrentChangeSequenceIntoUndoHistory()
                     onValuePostChange(BigTextChangeEventType.Delete, cursor, cursor + 1)
                     updateViewState()
                     if (log.config.minSeverity <= Severity.Verbose) {
                         (transformedText as BigTextImpl).printDebug("transformedText onDelete $direction")
                     }
+                    text.recordCurrentChangeSequenceIntoUndoHistory()
                     return true
                 }
             }
             TextFBDirection.Backward -> {
                 if (cursor - 1 >= 0) {
                     onValuePreChange(BigTextChangeEventType.Delete, cursor - 1, cursor)
                     text.delete(cursor - 1, cursor)
-                    text.recordCurrentChangeSequenceIntoUndoHistory()
                     onValuePostChange(BigTextChangeEventType.Delete, cursor - 1, cursor)
                     updateViewState()
                     if (log.config.minSeverity <= Severity.Verbose) {
@@ -576,16 +605,17 @@ private fun CoreBigMonospaceText(
                     viewState.updateTransformedCursorIndexByOriginal(transformedText)
                     viewState.transformedSelectionStart = viewState.transformedCursorIndex
                     log.v { "set cursor pos 3 => ${viewState.cursorIndex} t ${viewState.transformedCursorIndex}" }
+                    text.recordCurrentChangeSequenceIntoUndoHistory()
                     return true
                 }
             }
         }
         return false
     }
 
-    fun onUndoRedo(operation: (BigTextChangeCallback) -> Unit) {
+    fun onUndoRedo(operation: (BigTextChangeCallback) -> Pair<Boolean, Any?>) {
         var lastChangeEnd = -1
-        operation(object : BigTextChangeCallback {
+        val stateToBeRestored = operation(object : BigTextChangeCallback {
             override fun onValuePreChange(
                 eventType: BigTextChangeEventType,
                 changeStartIndex: Int,
@@ -606,7 +636,17 @@ private fun CoreBigMonospaceText(
                 }
             }
         })
-        if (lastChangeEnd >= 0) {
+        updateViewState()
+        (stateToBeRestored.second as? BigTextUndoMetadata)?.let { state ->
+            viewState.selection = state.selection
+            viewState.updateTransformedSelectionBySelection(transformedText)
+            viewState.cursorIndex = state.cursor
+            viewState.updateTransformedCursorIndexByOriginal(transformedText)
+            viewState.transformedSelectionStart = viewState.transformedCursorIndex
+            scrollToCursor()
+            return
+        }
+        if (lastChangeEnd >= 0) { // this `if` should never execute
             viewState.cursorIndex = lastChangeEnd
             viewState.updateTransformedCursorIndexByOriginal(transformedText)
             viewState.transformedSelectionStart = viewState.transformedCursorIndex
@@ -690,29 +730,9 @@ private fun CoreBigMonospaceText(
         return viewState.cursorIndex + wordBoundaryAt
     }
 
-    fun scrollToCursor() {
-        val layoutResult = layoutResult ?: return
-
-        // scroll to cursor position if out of visible range
-        val visibleVerticalRange = scrollState.value .. scrollState.value + height
-        val row = transformedText.findRowIndexByPosition(viewState.transformedCursorIndex)
-        val rowVerticalRange = layoutResult.getTopOfRow(row).toInt() .. layoutResult.getBottomOfRow(row).toInt()
-        if (rowVerticalRange !in visibleVerticalRange) {
-            val scrollToPosition = if (rowVerticalRange.start < visibleVerticalRange.start) {
-                rowVerticalRange.start
-            } else {
-                // scroll to a position that includes the bottom of the row + a little space
-                minOf(layoutResult.bottom.toInt(), maxOf(0, rowVerticalRange.endInclusive + maxOf(2, (layoutResult.rowHeight * 0.5).toInt()) - height))
-            }
-            coroutineScope.launch {
-                scrollState.animateScrollTo(scrollToPosition)
-            }
-        }
-    }
-
     fun updateOriginalCursorOrSelection(newPosition: Int, isSelection: Boolean) {
         val oldCursorPosition = viewState.cursorIndex
-        viewState.cursorIndex = newPosition // TODO scroll to new position
+        viewState.cursorIndex = newPosition
         viewState.updateTransformedCursorIndexByOriginal(transformedText)
         if (isSelection) {
             val selectionStart = if (viewState.hasSelection()) {

src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextImpl.kt

@@ -71,7 +71,12 @@ open class BigTextImpl(
 
     var decorator: BigTextDecorator? = null
 
+    var undoMetadataSupplier: (() -> Any?)? = null
+
+    private var currentUndoMetadata: Any? = null
+    private var currentRedoMetadata: Any? = null
     var currentChanges: MutableList<BigTextInputChange> = mutableListOf()
+        private set
     val undoHistory = CircularList<BigTextInputOperation>(undoHistoryCapacity)
     val redoHistory = CircularList<BigTextInputOperation>(undoHistoryCapacity)
 
@@ -419,6 +424,7 @@ open class BigTextImpl(
             leftStringLength = 0
         }
         if (isUndoEnabled) {
+            recordCurrentUndoMetadata()
             currentChanges += BigTextInputChange(
                 type = BigTextChangeEventType.Insert,
                 buffer = buffer,
@@ -957,6 +963,7 @@ open class BigTextImpl(
                 isD = true
             }
             if (isUndoEnabled) {
+                recordCurrentUndoMetadata()
                 currentChanges += BigTextInputChange(
                     type = BigTextChangeEventType.Delete,
                     buffer = node.value.buffer,
@@ -1031,9 +1038,18 @@ open class BigTextImpl(
         }
 
         if (currentChanges.isNotEmpty()) {
-            undoHistory.push(BigTextInputOperation(currentChanges.toList()))
+            undoHistory.push(BigTextInputOperation(currentChanges.toList(), currentUndoMetadata, undoMetadataSupplier?.invoke()))
             currentChanges = mutableListOf()
+            recordCurrentUndoMetadata()
+        }
+    }
+
+    protected fun recordCurrentUndoMetadata() {
+        if (currentChanges.isEmpty()) {
+            currentUndoMetadata = undoMetadataSupplier?.invoke()
         }
+        currentRedoMetadata = undoMetadataSupplier?.invoke()
+        log.d { "reset um = $currentUndoMetadata, rm = $currentRedoMetadata" }
     }
 
     protected fun clearRedoHistory() {
@@ -1110,33 +1126,42 @@ open class BigTextImpl(
         }
     }
 
-    fun undo(callback: BigTextChangeCallback? = null): Boolean {
+    fun undo(callback: BigTextChangeCallback? = null): Pair<Boolean, Any?> {
         if (!isUndoEnabled) {
-            return false
+            return false to null
         }
         if (currentChanges.isNotEmpty()) {
+            val undoMetadata = currentUndoMetadata
+            val redoMetadata = currentRedoMetadata
             applyReverseChangeSequence(currentChanges, callback)
-            redoHistory.push(BigTextInputOperation(currentChanges.toList()))
+            redoHistory.push(BigTextInputOperation(currentChanges.toList(), undoMetadata, redoMetadata))
             currentChanges = mutableListOf()
-            return true
+            recordCurrentUndoMetadata()
+            return true to undoMetadata
         }
-        val lastOperation = undoHistory.removeHead() ?: return false
+        val lastOperation = undoHistory.removeHead() ?: return false to null
         applyReverseChangeSequence(lastOperation.changes, callback)
+        currentUndoMetadata = lastOperation.undoMetadata
+        currentRedoMetadata = lastOperation.redoMetadata
+        log.d { "undo set um = $currentUndoMetadata, rm = $currentRedoMetadata" }
         redoHistory.push(lastOperation)
-        return true
+        return true to lastOperation.undoMetadata
     }
 
-    fun redo(callback: BigTextChangeCallback? = null): Boolean {
+    fun redo(callback: BigTextChangeCallback? = null): Pair<Boolean, Any?> {
         if (!isUndoEnabled) {
-            return false
+            return false to null
         }
         if (currentChanges.isNotEmpty()) { // should not happen
-            return false
+            return false to null
         }
-        val lastOperation = redoHistory.removeHead() ?: return false
+        val lastOperation = redoHistory.removeHead() ?: return false to null
         applyChangeSequence(lastOperation.changes, callback)
+        currentUndoMetadata = lastOperation.undoMetadata
+        currentRedoMetadata = lastOperation.redoMetadata
+        log.d { "undo set um = $currentUndoMetadata, rm = $currentRedoMetadata" }
         undoHistory.push(lastOperation)
-        return true
+        return true to lastOperation.redoMetadata
     }
 
     fun isUndoable(): Boolean = isUndoEnabled && (currentChanges.isNotEmpty() || undoHistory.isNotEmpty)

src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextInputChangeOperation.kt

@@ -3,7 +3,17 @@ package com.sunnychung.application.multiplatform.hellohttp.ux.bigtext
 import com.sunnychung.application.multiplatform.hellohttp.extension.length
 
 data class BigTextInputOperation(
-    val changes: List<BigTextInputChange>
+    val changes: List<BigTextInputChange>,
+
+    /**
+     * Metadata to apply if changes of this operation have been reversed.
+     */
+    val undoMetadata: Any?,
+
+    /**
+     * Metadata to apply if changes of this operation have been applied.
+     */
+    val redoMetadata: Any?,
 )
 
 data class BigTextInputChange(

src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextUndoMetadata.kt

@@ -0,0 +1,6 @@
+package com.sunnychung.application.multiplatform.hellohttp.ux.bigtext
+
+data class BigTextUndoMetadata(
+    val cursor: Int,
+    val selection: IntRange,
+)

src/jvmMain/kotlin/com/sunnychung/application/multiplatform/hellohttp/ux/bigtext/BigTextViewState.kt

@@ -50,8 +50,12 @@ class BigTextViewState {
     }
 
     internal fun updateTransformedSelectionBySelection(transformedText: BigTextTransformed) {
-        transformedSelection = transformedText.findTransformedPositionByOriginalPosition(selection.first) ..
-                transformedText.findTransformedPositionByOriginalPosition(selection.last)
+        transformedSelection = if (!selection.isEmpty()) {
+            transformedText.findTransformedPositionByOriginalPosition(selection.first) ..
+                    transformedText.findTransformedPositionByOriginalPosition(selection.last)
+        } else {
+            IntRange.EMPTY
+        }
     }
 
     internal var transformedCursorIndex by mutableStateOf(0)

src/jvmTest/kotlin/com/sunnychung/application/multiplatform/hellohttp/test/bigtext/BigTextUndoRedoTest.kt

@@ -121,15 +121,15 @@ class BigTextUndoRedoTest {
         t.delete(6 .. 6)
         assertEquals("abcdef", t.buildString())
         listOf("abcdefg", "abcd").forEach { expected ->
-            assertEquals(true, t.undo())
+            assertEquals(true, t.undo().first)
             assertEquals(expected, t.buildString())
         }
 
         // no redo after making a change
         t.append("x")
         assertEquals("abcdx", t.buildString())
         (1..10).forEach {
-            assertEquals(false, t.redo())
+            assertEquals(false, t.redo().first)
             assertEquals("abcdx", t.buildString())
         }
 
@@ -139,27 +139,27 @@ class BigTextUndoRedoTest {
     fun assertUndoRedoUndo(reversedExpectedStrings: List<String>, t: BigTextImpl) {
         assertEquals(reversedExpectedStrings.first(), t.buildString())
         reversedExpectedStrings.stream().skip(1).forEach { expected ->
-            assertEquals(true, t.undo())
+            assertEquals(true, t.undo().first)
             assertEquals(expected, t.buildString())
         }
         (1..3).forEach {
-            assertEquals(false, t.undo())
+            assertEquals(false, t.undo().first)
             assertEquals(reversedExpectedStrings.last(), t.buildString())
         }
         reversedExpectedStrings.asReversed().stream().skip(1).forEach { expected ->
-            assertEquals(true, t.redo())
+            assertEquals(true, t.redo().first)
             assertEquals(expected, t.buildString())
         }
         (1..10).forEach {
-            assertEquals(false, t.redo())
+            assertEquals(false, t.redo().first)
             assertEquals(reversedExpectedStrings.first(), t.buildString())
         }
         reversedExpectedStrings.stream().skip(1).forEach { expected ->
-            assertEquals(true, t.undo())
+            assertEquals(true, t.undo().first)
             assertEquals(expected, t.buildString())
         }
         (1..10).forEach {
-            assertEquals(false, t.undo())
+            assertEquals(false, t.undo().first)
             assertEquals(reversedExpectedStrings.last(), t.buildString())
         }
     }