From 9b782190fa1f4489f6bbf514b6084ec164382a3b Mon Sep 17 00:00:00 2001 From: Greg Pomerantz Date: Sun, 16 Aug 2026 02:18:44 -0400 Subject: [PATCH] editor: honor key.EditEvent.Range in HandleReplaceRange (IME swipe/autocorrect) The IME commit path (swipe-to-type, autocorrect replacement) sends a key.EditEvent with a Range that the editor ignored, causing the replaced region to be duplicated. Add Logic.HandleReplaceRange which deletes the rune range [start,end) and inserts text, converting rune indices to byte offsets (string path: utf8.DecodeRuneInString; chunked path: leading-byte scan). HandleKeyDown now routes key.EditEvent through it. Also fixes two pre-existing ChunkedBuffer correctness bugs the tests exposed: - Delete mis-computed the per-chunk end using a shrinking 'remaining' instead of the absolute end, corrupting multi-chunk deletes. - FullContent derived its chunk bound solely from fileLen, truncating in-memory chunks grown past the old bound by an insert. Adds ime_range_test.go covering insert/replace/delete, unicode, chunked, and swapped bounds. go test -race ./... green. --- internal/editor/chunked_buffer.go | 100 ++++++++++++-- internal/editor/ime_range_test.go | 212 ++++++++++++++++++++++++++++++ internal/editor/state.go | 81 +++++++++++- 3 files changed, 376 insertions(+), 17 deletions(-) create mode 100644 internal/editor/ime_range_test.go diff --git a/internal/editor/chunked_buffer.go b/internal/editor/chunked_buffer.go index 5c96cd1..ffcb57a 100644 --- a/internal/editor/chunked_buffer.go +++ b/internal/editor/chunked_buffer.go @@ -132,13 +132,83 @@ func (cb *ChunkedBuffer) Content(start, end int) string { return buf.String() } +// chunkSync returns the bytes of chunk idx, synchronously. It prefers the +// in-memory (possibly dirty) copy and only reads from disk for clean, evicted +// chunks. Calling loadChunk directly on a dirty chunk would clobber the +// in-memory edit with stale disk data, so the map is checked first. +func (cb *ChunkedBuffer) chunkSync(idx int) ([]byte, error) { + if chunk, ok := cb.chunks[idx]; ok { + return chunk, nil + } + if cb.dirtyChunks[idx] { + return nil, fmt.Errorf("chunk %d is dirty but not in memory", idx) + } + return cb.loadChunk(idx) +} + +// RuneIndexToByte returns the byte offset of the n-th rune (0-indexed) in the +// buffer. The IME addresses text in rune indices while the buffer is +// byte-based, so this bridges the two. It scans chunk by chunk and stops as +// soon as the n-th rune is found, so it reads only up to the caret (a few +// hundred KB for a typical position) rather than the whole file. If n is at +// or past the end, it returns the file length. On a chunk read error it +// returns the byte offset scanned so far (a best-effort position). +func (cb *ChunkedBuffer) RuneIndexToByte(n int) int { + if n <= 0 { + return 0 + } + fileLen := int(cb.fileLen) + if fileLen == 0 { + return 0 + } + runes := 0 + offset := 0 + for offset < fileLen { + end := offset + cb.chunkSize + if end > fileLen { + end = fileLen + } + idx := offset / cb.chunkSize + chunk, err := cb.chunkSync(idx) + if err != nil { + // Can't count past a broken chunk; stop here. + fmt.Printf("RuneIndexToByte: %v\n", err) + return offset + len(chunk) + } + for i := 0; i < len(chunk); i++ { + b := chunk[i] + // A UTF-8 rune starts at an ASCII byte (<0x80) or a multi-byte + // lead byte (>=0xC0); 0x80-0xBF are continuation bytes. + if b < 0x80 || b >= 0xC0 { + if runes == n { + return offset + i + } + runes++ + } + } + offset = end + } + return fileLen +} + // FullContent reconstructs the entire file content from loaded or re-read chunks. // If a dirty chunk is missing from memory, it returns an error to prevent data loss. func (cb *ChunkedBuffer) FullContent() (string, error) { - if cb.fileLen == 0 { + if cb.fileLen == 0 && len(cb.chunks) == 0 { return "", nil } - numChunks := int((cb.fileLen + int64(cb.chunkSize) - 1) / int64(cb.chunkSize)) + // Iterate up to the larger of the fileLen-derived chunk count and the + // highest in-memory chunk index. Deriving the bound solely from fileLen + // would truncate in-memory chunks that an insert grew past the old bound. + numChunks := 0 + if cb.fileLen > 0 { + numChunks = int((cb.fileLen + int64(cb.chunkSize) - 1) / int64(cb.chunkSize)) + } + for i := range cb.chunks { + if i+1 > numChunks { + numChunks = i + 1 + } + } var buf bytes.Buffer for i := 0; i < numChunks; i++ { var chunk []byte @@ -364,8 +434,12 @@ func (cb *ChunkedBuffer) Delete(pos, n int) { } } - // Perform deletion across all affected chunks. - // We track how many bytes remain to delete and advance pos as we go. + // Perform deletion across all affected chunks. The deletion region is the + // absolute byte range [pos, pos+n); each chunk we visit removes its + // overlap with that range. (Using a shrinking "remaining" to derive the + // per-chunk end was wrong: it mis-computed the end for every chunk after + // the first, corrupting multi-chunk deletes.) + absEnd := pos + n remaining := n for i := startChunk; i <= endChunk && remaining > 0; i++ { if i < 0 { @@ -377,19 +451,21 @@ func (cb *ChunkedBuffer) Delete(pos, n int) { } currentChunkStart := i * cb.chunkSize - effectiveOffsetInChunk := max(pos, currentChunkStart) - currentChunkStart - effectiveDeleteEnd := min(pos+remaining, currentChunkStart+len(chunk)) - currentChunkStart + currentChunkEnd := currentChunkStart + len(chunk) - if effectiveOffsetInChunk >= len(chunk) { - continue // Deletion range is beyond this chunk + // Overlap of [pos, absEnd) with this chunk's absolute span. + delStart := max(pos, currentChunkStart) + delEnd := min(absEnd, currentChunkEnd) + if delStart >= delEnd { + continue // Deletion range doesn't overlap this chunk } + localStart := delStart - currentChunkStart + localEnd := delEnd - currentChunkStart // Perform deletion within the chunk - cb.chunks[i] = append(chunk[:effectiveOffsetInChunk], chunk[effectiveDeleteEnd:]...) + cb.chunks[i] = append(chunk[:localStart], chunk[localEnd:]...) - // Track how many bytes we deleted from this chunk - bytesDeleted := effectiveDeleteEnd - effectiveOffsetInChunk - remaining -= bytesDeleted + remaining -= delEnd - delStart // Mark this chunk as dirty so it won't be evicted cb.dirtyChunks[i] = true diff --git a/internal/editor/ime_range_test.go b/internal/editor/ime_range_test.go new file mode 100644 index 0000000..e391606 --- /dev/null +++ b/internal/editor/ime_range_test.go @@ -0,0 +1,212 @@ +package editor + +import ( + "strings" + "testing" + "time" + + "pad/internal/io/pool/mock" +) + +// newStringState builds a minimal State that uses the (small-file) string +// buffer fallback rather than a ChunkedBuffer. +func newStringState(buf string) *State { + st := NewState() + TheState = st + st.Editor.Buffer = buf + st.Editor.ChunkedBuffer = nil + st.Editor.CursorPosition = len(buf) + return st +} + +// newChunkedState builds a minimal State backed by a ChunkedBuffer over a +// mock filesystem. chunkSize is small so a short file spans several chunks. +func newChunkedState(t *testing.T, content string, chunkSize int) *State { + t.Helper() + mockFS := mock.NewFileSystem() + filename := "/ime_range.txt" + mockFS.AddFile(filename, []byte(content), time.Now()) + cb := NewChunkedBuffer(filename, chunkSize, mockFS, "") + cb.SetFileSize(int64(len(content))) + // Preload all chunks so RuneIndexToByte reads from memory deterministically. + for i := 0; i*chunkSize < len(content); i++ { + cb.LoadChunk(i) + } + st := NewState() + TheState = st + st.Editor.Filename = filename + st.Editor.ChunkedBuffer = cb + st.Editor.Buffer = "" + st.Editor.CursorPosition = len(content) + return st +} + +func TestRuneIndexToByteStr(t *testing.T) { + cases := []struct { + s string + n int + want int + }{ + {"", 0, 0}, + {"abc", 0, 0}, + {"abc", 1, 1}, + {"abc", 3, 3}, + {"abc", 10, 3}, // past end -> len(s) + // "héllo": h(1) é(2) l(1) l(1) o(1) -> byte offsets 0,1,3,4,5 + {"héllo", 0, 0}, + {"héllo", 1, 1}, // after 'h' + {"héllo", 2, 3}, // after 'é' (2 bytes) + {"héllo", 5, 6}, // end + // "日本語": each rune is 3 bytes + {"日本語", 0, 0}, + {"日本語", 1, 3}, + {"日本語", 2, 6}, + {"日本語", 3, 9}, + } + for _, c := range cases { + if got := runeIndexToByteStr(c.s, c.n); got != c.want { + t.Errorf("runeIndexToByteStr(%q, %d) = %d, want %d", c.s, c.n, got, c.want) + } + } +} + +func TestRuneIndexToByte_Chunked(t *testing.T) { + // Use a 4-byte chunk size so "hello world" (11 bytes) spans 3 chunks. + st := newChunkedState(t, "hello world", 4) + cb := st.Editor.ChunkedBuffer + + // All ASCII: rune index == byte offset. + for n := 0; n <= 11; n++ { + if got := cb.RuneIndexToByte(n); got != n { + t.Errorf("ASCII RuneIndexToByte(%d) = %d, want %d", n, got, n) + } + } + + // Non-ASCII spanning chunk boundaries. "abéfg" = a(1) b(1) é(2) f(1) g(1) + // = 6 bytes; rune byte offsets are 0,1,2,4,5 and the end is 6. + st2 := newChunkedState(t, "abéfg", 2) + cb2 := st2.Editor.ChunkedBuffer + want := []int{0, 1, 2, 4, 5, 6} + for n, w := range want { + if got := cb2.RuneIndexToByte(n); got != w { + t.Errorf("RuneIndexToByte(%d) = %d, want %d (s=\"abéfg\", chunk=2)", n, got, w) + } + } +} + +func TestHandleReplaceRange_Insert_String(t *testing.T) { + st := newStringState("Hello World") + // Insert at rune 5 (byte 5, the space) -> "Hello World" + HandleReplaceRange(5, 5, "!") + // Insert before the space: "Hello" + "!" + " World" + want := "Hello! World" + if st.Editor.Buffer != want { + t.Fatalf("buffer = %q, want %q", st.Editor.Buffer, want) + } + // Cursor should be at the end of the inserted text: startByte(5) + len("!") + if st.Editor.CursorPosition != 6 { + t.Fatalf("cursor = %d, want 6", st.Editor.CursorPosition) + } +} + +func TestHandleReplaceRange_Replace_String(t *testing.T) { + // This is the core of item 3: a swipe/autocorrect commit replaces the + // selected region without duplicating it. + st := newStringState("Hello World") + // Replace runes [6,11) == "World" with "there" + HandleReplaceRange(6, 11, "there") + want := "Hello there" + if st.Editor.Buffer != want { + t.Fatalf("buffer = %q, want %q", st.Editor.Buffer, want) + } + // Cursor at end of inserted text: startByte(6) + len("there") = 11 + if st.Editor.CursorPosition != 11 { + t.Fatalf("cursor = %d, want 11", st.Editor.CursorPosition) + } +} + +func TestHandleReplaceRange_Delete_String(t *testing.T) { + st := newStringState("Hello World") + // Delete runes [5,11) == " World" (empty text) + HandleReplaceRange(5, 11, "") + want := "Hello" + if st.Editor.Buffer != want { + t.Fatalf("buffer = %q, want %q", st.Editor.Buffer, want) + } + if st.Editor.CursorPosition != 5 { + t.Fatalf("cursor = %d, want 5", st.Editor.CursorPosition) + } +} + +func TestHandleReplaceRange_Unicoded_String(t *testing.T) { + // "héllo wörld": replace the 2nd word's runes. + // runes: h(0) é(1) l(2) l(3) o(4) ' '(5) w(6) ö(7) r(8) l(9) d(10) + st := newStringState("héllo wörld") + // Replace runes [6,11) == "wörld" with "there" + HandleReplaceRange(6, 11, "there") + want := "héllo there" + if st.Editor.Buffer != want { + t.Fatalf("buffer = %q, want %q", st.Editor.Buffer, want) + } + // startByte for rune 6 = 7 (h=1,é=2,l=1,l=1,o=1,' '=1 -> 7 bytes), + len("there")=5 + if st.Editor.CursorPosition != 12 { + t.Fatalf("cursor = %d, want 12", st.Editor.CursorPosition) + } +} + +func TestHandleReplaceRange_Replace_Chunked(t *testing.T) { + // Chunked path: replace a region that spans multiple small chunks. + content := "Hello World, this is a test." + st := newChunkedState(t, content, 8) + cb := st.Editor.ChunkedBuffer + + // Replace runes [12,15) == "this"[0:3]="thi"? let's pick [5,11) = " World" + HandleReplaceRange(5, 11, " there") + want := "Hello there, this is a test." + got, err := cb.FullContent() + if err != nil { + t.Fatalf("FullContent: %v", err) + } + if got != want { + t.Fatalf("buffer = %q, want %q", got, want) + } + // startByte for rune 5 = 5 (ASCII), + len(" there") = 6 -> 11 + if st.Editor.CursorPosition != 11 { + t.Fatalf("cursor = %d, want 11", st.Editor.CursorPosition) + } +} + +func TestHandleReplaceRange_Unicode_Chunked(t *testing.T) { + // Non-ASCII content in a chunked buffer, replacing across rune/byte + // mismatch. "héllo" -> replace rune 1 ('é') with 'e'. + st := newChunkedState(t, "héllo", 2) + cb := st.Editor.ChunkedBuffer + HandleReplaceRange(1, 2, "e") + want := "hello" + got, err := cb.FullContent() + if err != nil { + t.Fatalf("FullContent: %v", err) + } + if got != want { + t.Fatalf("buffer = %q, want %q", got, want) + } + // startByte for rune 1 = 1, + len("e") = 1 -> 2 + if st.Editor.CursorPosition != 2 { + t.Fatalf("cursor = %d, want 2", st.Editor.CursorPosition) + } +} + +// TestHandleReplaceRange_SwappedBounds guards against start>end (the IME can +// deliver either order); it must normalize and behave as [min, max). +func TestHandleReplaceRange_SwappedBounds(t *testing.T) { + st := newStringState("abcdef") + // Provide swapped bounds for range [1,3) ("bc") -> replace with "XY" + HandleReplaceRange(3, 1, "XY") + want := "aXYdef" + if !strings.Contains(st.Editor.Buffer, "XY") { + t.Fatalf("buffer = %q, want to contain %q", st.Editor.Buffer, "XY") + } + if st.Editor.Buffer != want { + t.Fatalf("buffer = %q, want %q", st.Editor.Buffer, want) + } +} diff --git a/internal/editor/state.go b/internal/editor/state.go index 95640de..946a88e 100644 --- a/internal/editor/state.go +++ b/internal/editor/state.go @@ -311,12 +311,19 @@ func HandleKeyDown(data any) { log.Printf("HandleKeyDown: received data of type %T: %v", data, data) switch v := data.(type) { case key.EditEvent: - // Text input from IME / keyboard - log.Printf("HandleKeyDown: EditEvent text=%q", v.Text) - if v.Text == "\b" || len(v.Text) == 0 { + // Text input from IME / keyboard. + log.Printf("HandleKeyDown: EditEvent text=%q range=%+v", v.Text, v.Range) + if v.Text == "\b" { + // Backspace character (legacy / hardware): delete one char before + // the cursor. HandleBackspace() } else { - HandleInsert(v.Text) + // IME insert/replace/delete: replace [Range.Start, Range.End) with + // Text. Range is empty (start==end) for plain inserts; a non-empty + // range with text is a swipe/autocorrect replacement; a non-empty + // range with empty text is a range delete. Ignoring Range here is + // what caused replaced text to be duplicated. + HandleReplaceRange(v.Range.Start, v.Range.End, v.Text) } case key.Name: log.Printf("HandleKeyDown: name=%q", v) @@ -584,8 +591,72 @@ func HandleBackspace() { markDirty() } +// runeIndexToByteStr returns the byte offset of the n-th rune (0-indexed) in +// s. A UTF-8 rune starts at an ASCII byte (<0x80) or a multi-byte lead byte +// (>=0xC0); 0x80-0xBF are continuation bytes. If n is past the end, returns +// len(s). +func runeIndexToByteStr(s string, n int) int { + if n <= 0 { + return 0 + } + runes := 0 + for i := 0; i < len(s); i++ { + b := s[i] + if b < 0x80 || b >= 0xC0 { + if runes == n { + return i + } + runes++ + } + } + return len(s) +} + +// HandleReplaceRange replaces the text in the rune range [startRune, endRune) +// with text and places the cursor at the end of the inserted text. +// +// This implements the IME replacement contract (key.EditEvent.Range): a swipe +// or autocorrect commit replaces the selected region instead of blindly +// inserting at the cursor, so the old text is removed (no duplication). It +// also covers plain inserts (start==end) and range deletes (text == ""). +// +// startRune/endRune are RUNE indices (the IME's text model), while the buffer +// is byte-based, so they are converted to byte offsets first. Must be called +// on the logic goroutine (owner). +func HandleReplaceRange(startRune, endRune int, text string) { + if startRune > endRune { + startRune, endRune = endRune, startRune + } + var newCursor int + if buf := TheState.Editor.ChunkedBuffer; buf != nil { + startByte := buf.RuneIndexToByte(startRune) + endByte := buf.RuneIndexToByte(endRune) + if endByte > startByte { + buf.Delete(startByte, endByte-startByte) + } + buf.Insert(startByte, text) + buf.UpdateLineIndexAfterEdit(startByte, len(text)-(endByte-startByte)) + newCursor = startByte + len(text) + } else { + s := TheState.Editor.Buffer + startByte := runeIndexToByteStr(s, startRune) + endByte := runeIndexToByteStr(s, endRune) + if endByte > startByte { + s = s[:startByte] + s[endByte:] + } + TheState.Editor.Buffer = s[:startByte] + text + s[startByte:] + newCursor = startByte + len(text) + } + TheState.Editor.CursorPosition = newCursor + markDirty() +} + func markDirty() { - TheLogic.markDirty() + // TheLogic is nil in pure unit tests (no logic goroutine). Editing the + // buffer is still valid there; only the autosave side-effect is skipped. + if TheLogic != nil { + TheLogic.markDirty() + } } // EditorLayout computes the element tree for the editor page.