From 3460ef399302e8bcf3d59988033668d697b4cf32 Mon Sep 17 00:00:00 2001 From: Greg Pomerantz Date: Sun, 16 Aug 2026 10:35:10 -0400 Subject: [PATCH] editor: fix viewport-on-open, chunked-buffer drift, add size guard (Phase 3 code) - Viewport: swallow the opening tap/scroll (justOpenedAt window) and re-clamp ScrollOffset to [0,MaxScroll] in EditorLayout so a short file never opens past its content (blank viewport). - Chunked buffer: replace the fixed i*chunkSize slot model (which drifted after length-changing edits and could re-read stale disk for shifted tail chunks) with an ordered chunk slice + prefix-sum byte offsets. In-range files now load fully on open (SetContent), so there is no lazy load and no stale-disk re-read. Edits splice only the affected chunk(s). - Size guard: files > MaxEditableFileSize (50 MB) show a 'too large to edit' notice instead of loading; the browser still lists them. Edit handlers (KeyDown/ReplaceRange) are no-ops for too-large files. - Tests: rewrite chunked_buffer_test.go for prefix-sum correctness (insert/ delete across chunk boundaries, rune->byte after edit); fix the large-file e2e expectation to the shift-correct ground truth. --- internal/editor/chunked_buffer.go | 590 ++++++++++--------------- internal/editor/chunked_buffer_test.go | 301 +++++++------ internal/editor/e2e_test.go | 49 +- internal/editor/ime_range_test.go | 7 +- internal/editor/logic.go | 27 +- internal/editor/state.go | 84 +++- 6 files changed, 502 insertions(+), 556 deletions(-) diff --git a/internal/editor/chunked_buffer.go b/internal/editor/chunked_buffer.go index ffcb57a..b8e8b49 100644 --- a/internal/editor/chunked_buffer.go +++ b/internal/editor/chunked_buffer.go @@ -2,7 +2,6 @@ package editor import ( "bytes" - "fmt" "sort" "pad/internal/io/pool" @@ -12,38 +11,61 @@ import ( const ( DefaultChunkSize = 64 * 1024 // 64 KB + // MaxEditableFileSize is the largest file the editor will open for editing. + // In-range files are loaded fully into memory (see Phase 3): the chunked + // buffer keeps the raw bytes (~file size) resident and the renderer + // virtualizes the glyph layout to the visible window. Files above this are + // rejected with a "too large to edit" state (the browser can still list + // them). Set from on-device measurement (see doc/development_plan.md §5). + MaxEditableFileSize = 50 * 1024 * 1024 // 50 MB ) // ChunkedBuffer provides chunked access to a file's content. -// Only chunks near the cursor or viewport are kept in memory. +// +// Model (Phase 3): for in-range files the ENTIRE file is loaded into memory on +// open and split into ordered chunks (see SetContent). Each chunk keeps its +// own length; the byte offset of a chunk is the sum of the lengths of the +// chunks before it (a prefix sum), NOT a fixed i*chunkSize slot. This is what +// makes edits correct: an insert/delete changes a chunk's length and the +// prefix sums automatically shift every later chunk, so byte->chunk mapping +// stays exact. Because every chunk is resident, there is no lazy loading and +// therefore no stale-disk re-read (the old fixed-slot model could re-read a +// shifted tail chunk from disk and clobber in-memory edits). +// +// Chunks may grow/shrink with edits and are not rebalanced; that is fine for +// correctness (prefix sums) and keeps edits local to the affected chunk(s) +// rather than copying the whole file. type ChunkedBuffer struct { filename string - chunkSize int // e.g., 64 * 1024 (64 KB) - fileLen int64 // total file length (known from stat) - chunks map[int][]byte // chunkIndex → []byte - dirty bool // true if buffer has been modified - FS pool.FileSystem // filesystem for reading chunks + chunkSize int // target chunk size (e.g. 64 KB); actual chunks may vary + fileLen int64 // total content length + chunks [][]byte // ordered; chunks[i] is the i-th chunk + dirty bool // true if buffer has been modified + FS pool.FileSystem // filesystem for reads basePath string // base path for file resolution // line index is built asynchronously LineIndex *types.LineIndex - // workerPool is set by the logic goroutine after the buffer is created. - // Used for async prefetch of chunks. + // workerPool is retained for API compatibility; in-range files load fully + // up front, so chunk loading no longer dispatches worker tasks. workerPool *pool.WorkerPool - // lastPrefetchedChunk tracks the last chunk that was prefetched, - // so we can avoid redundant prefetches on the same position. + // lastPrefetchedChunk tracks the last chunk that was prefetched, so callers + // can avoid redundant work. Kept for compatibility. lastPrefetchedChunk int - // dirtyChunks tracks which individual chunks have been modified - // since they were last persisted to disk. This prevents eviction - // of modified chunks, which would otherwise lose edits. + // dirtyChunks tracks which individual chunks have been modified since they + // were last persisted to disk. dirtyChunks map[int]bool - // loadingChunks tracks which chunks are currently being loaded - // to avoid dispatching redundant tasks for the same chunk. + // loadingChunks is retained for API compatibility; it is always empty for + // in-range files (no lazy loading). loadingChunks map[int]bool + + // fullyLoaded is true once SetContent has populated all chunks; the buffer + // never falls back to disk afterwards. + fullyLoaded bool } // NewChunkedBuffer creates a new ChunkedBuffer. @@ -54,7 +76,8 @@ func NewChunkedBuffer(filename string, chunkSize int, fs pool.FileSystem, basePa return &ChunkedBuffer{ filename: filename, chunkSize: chunkSize, - chunks: make(map[int][]byte), + fileLen: 0, + chunks: nil, dirtyChunks: make(map[int]bool), loadingChunks: make(map[int]bool), FS: fs, @@ -62,14 +85,15 @@ func NewChunkedBuffer(filename string, chunkSize int, fs pool.FileSystem, basePa } } -// SetFileSize sets the total file length. +// SetFileSize sets the total file length (used before SetContent, e.g. from a +// stat). It only applies when the buffer is not dirty. func (cb *ChunkedBuffer) SetFileSize(length int64) { if !cb.dirty { cb.fileLen = length } } -// FileLen returns the total file length. +// FileLen returns the total content length. func (cb *ChunkedBuffer) FileLen() int64 { return cb.fileLen } @@ -79,18 +103,60 @@ func (cb *ChunkedBuffer) Filename() string { return cb.filename } -// ChunkSize returns the chunk size. +// ChunkSize returns the target chunk size. func (cb *ChunkedBuffer) ChunkSize() int { return cb.chunkSize } -// Content returns the bytes in [start, end) from the chunked buffer. -// It loads missing chunks on demand. +// SetContent replaces the buffer content with data, split into ordered chunks +// of at most chunkSize. This is the on-open load path for in-range files: the +// whole file becomes resident, so there is no lazy loading and no risk of +// re-reading stale disk data after an edit. +func (cb *ChunkedBuffer) SetContent(data []byte) { + cb.chunks = nil + for start := 0; start < len(data); start += cb.chunkSize { + end := start + cb.chunkSize + if end > len(data) { + end = len(data) + } + chunk := make([]byte, end-start) + copy(chunk, data[start:end]) + cb.chunks = append(cb.chunks, chunk) + } + if len(data) == 0 { + cb.chunks = [][]byte{} + } + cb.fileLen = int64(len(data)) + cb.dirty = false + cb.dirtyChunks = make(map[int]bool) + cb.fullyLoaded = true +} + +// chunkForPos returns the index of the chunk that contains byte offset pos and +// the offset of pos within that chunk. If pos is at or past the end, it returns +// the last chunk and an offset at its end (so callers can append). If the +// buffer is empty it returns (-1, 0). +func (cb *ChunkedBuffer) chunkForPos(pos int) (int, int) { + offset := 0 + for i, chunk := range cb.chunks { + if pos < offset+len(chunk) { + return i, pos - offset + } + offset += len(chunk) + } + if n := len(cb.chunks); n > 0 { + return n - 1, len(cb.chunks[n-1]) + } + return -1, 0 +} + +// Content returns the bytes in [start, end) from the chunked buffer. Chunks are +// walked by their actual (prefix-sum) offsets, so the mapping stays correct +// after length-changing edits. func (cb *ChunkedBuffer) Content(start, end int) string { if cb.fileLen == 0 { return "" } - // Clamp range to file bounds if start < 0 { start = 0 } @@ -100,31 +166,20 @@ func (cb *ChunkedBuffer) Content(start, end int) string { if start >= end { return "" } - - startChunk := start / cb.chunkSize - endChunk := (end - 1) / cb.chunkSize - var buf bytes.Buffer - for i := startChunk; i <= endChunk; i++ { - chunk, ok := cb.chunks[i] - if !ok { - // If chunk is not loaded, initiate async load and skip this chunk. - // This keeps the UI responsive while chunks are loaded in the background. - if cb.workerPool != nil && !cb.loadingChunks[i] { - cb.loadingChunks[i] = true - cb.workerPool.DispatchNonBlocking( - pool.NewReadChunkTask(cb.filename, i, cb.FS), - ) - } + offset := 0 + for _, chunk := range cb.chunks { + chunkStart := offset + chunkEnd := offset + len(chunk) + offset = chunkEnd + if chunkEnd <= start { continue } - - chunkStart := i * cb.chunkSize - chunkEnd := chunkStart + len(chunk) - + if chunkStart >= end { + break + } segStart := max(start, chunkStart) - chunkStart segEnd := min(end, chunkEnd) - chunkStart - if segStart < segEnd { buf.Write(chunk[segStart:segEnd]) } @@ -132,49 +187,21 @@ 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). +// byte-based, so this bridges the two. It walks chunks by actual length and +// stops as soon as the n-th rune is found. If n is at or past the end it +// returns the content length. func (cb *ChunkedBuffer) RuneIndexToByte(n int) int { if n <= 0 { return 0 } - fileLen := int(cb.fileLen) - if fileLen == 0 { + if cb.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 _, chunk := range cb.chunks { for i := 0; i < len(chunk); i++ { b := chunk[i] // A UTF-8 rune starts at an ASCII byte (<0x80) or a multi-byte @@ -186,91 +213,69 @@ func (cb *ChunkedBuffer) RuneIndexToByte(n int) int { runes++ } } - offset = end + offset += len(chunk) } - return fileLen + return int(cb.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. +// FullContent reconstructs the entire content by concatenating the ordered +// chunks. For in-range files every chunk is resident, so this is exact and +// never returns an error. func (cb *ChunkedBuffer) FullContent() (string, error) { - if cb.fileLen == 0 && len(cb.chunks) == 0 { - return "", nil - } - // 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 - var err error - if loadedChunk, ok := cb.chunks[i]; ok { - chunk = loadedChunk - } else { - // If chunk is dirty but not in memory, we have a problem. - if cb.dirtyChunks[i] { - return "", fmt.Errorf("critical error: dirty chunk %d is missing from memory", i) - } - // Re-read from disk if not in memory - chunk, err = cb.loadChunk(i) - if err != nil { - fmt.Printf("Error re-reading chunk %d for file %s during FullContent: %v\n", i, cb.filename, err) - continue // Skip this chunk on error - } - } + for _, chunk := range cb.chunks { buf.Write(chunk) } return buf.String(), nil } -// loadChunk reads a specific chunk from disk and returns its content. -// It also stores the chunk in the 'chunks' map. -// Uses FS.ReadFileAt to read only the chunk range (not the entire file). +// loadChunk reads a chunk from disk. Retained for compatibility; in-range +// files load fully via SetContent and do not use this. func (cb *ChunkedBuffer) loadChunk(idx int) ([]byte, error) { start := idx * cb.chunkSize chunk, err := cb.FS.ReadFileAt(cb.filename, start, cb.chunkSize) if err != nil { - return nil, fmt.Errorf("failed to read chunk %d: %w", idx, err) + return nil, err } - cb.chunks[idx] = chunk // Store the loaded chunk + for len(cb.chunks) <= idx { + cb.chunks = append(cb.chunks, nil) + } + cb.chunks[idx] = chunk return chunk, nil } -// LoadChunk explicitly loads a chunk and prefetch adjacent ones. +// LoadChunk loads a chunk (no-op for fully-loaded in-range files). func (cb *ChunkedBuffer) LoadChunk(idx int) { - if _, ok := cb.chunks[idx]; !ok { - _, err := cb.loadChunk(idx) - if err != nil { - fmt.Printf("Error loading chunk %d: %v\n", idx, err) - } + if cb.fullyLoaded { + return } - // Prefetch adjacent chunks - cb.Prefetch(idx, 1) + if idx >= 0 && idx < len(cb.chunks) && cb.chunks[idx] != nil { + return + } + if _, err := cb.loadChunk(idx); err != nil { + // Best effort; in-range files should never reach here. + } + cb.lastPrefetchedChunk = idx } -// IsChunkLoaded returns true if the chunk is already in memory. +// IsChunkLoaded reports whether the chunk is resident. func (cb *ChunkedBuffer) IsChunkLoaded(idx int) bool { - _, ok := cb.chunks[idx] - return ok + return idx >= 0 && idx < len(cb.chunks) && cb.chunks[idx] != nil } -// IsChunkLoading returns true if the chunk is currently being loaded. +// IsChunkLoading reports whether a chunk load is in flight (never, for +// in-range files). func (cb *ChunkedBuffer) IsChunkLoading(idx int) bool { return cb.loadingChunks[idx] } -// LoadChunkAsync dispatches an async task to load a chunk. +// LoadChunkAsync dispatches an async chunk load (no-op for in-range files, +// which are fully resident). func (cb *ChunkedBuffer) LoadChunkAsync(idx int) { - if cb.workerPool != nil && !cb.loadingChunks[idx] { + if cb.fullyLoaded { + return + } + if cb.workerPool != nil && !cb.loadingChunks[idx] && !cb.IsChunkLoaded(idx) { cb.loadingChunks[idx] = true cb.workerPool.DispatchNonBlocking( pool.NewReadChunkTask(cb.filename, idx, cb.FS), @@ -278,211 +283,112 @@ func (cb *ChunkedBuffer) LoadChunkAsync(idx int) { } } -// SetWorkerPool sets the worker pool for async chunk loading. +// SetWorkerPool sets the worker pool (retained for compatibility). func (cb *ChunkedBuffer) SetWorkerPool(wp *pool.WorkerPool) { cb.workerPool = wp } -// LastPrefetchedChunk returns the last chunk that was prefetched. -// Used by EditorLayout to avoid redundant prefetches. +// LastPrefetchedChunk returns the last prefetched chunk index. func (cb *ChunkedBuffer) LastPrefetchedChunk() int { return cb.lastPrefetchedChunk } -// Prefetch loads adjacent chunks for smooth scrolling. -// radius is the number of chunks to load on each side. -// Uses async ReadChunkTask when a worker pool is available to avoid blocking. +// Prefetch is a no-op for in-range files (all chunks resident). func (cb *ChunkedBuffer) Prefetch(centerChunk int, radius int) { - numChunks := int((cb.fileLen + int64(cb.chunkSize) - 1) / int64(cb.chunkSize)) - for i := centerChunk - radius; i <= centerChunk + radius; i++ { - if i >= 0 && i < numChunks { - // Check if chunk is in memory or already loading. - if _, ok := cb.chunks[i]; !ok && !cb.loadingChunks[i] { - if cb.workerPool != nil { - // Mark as loading before dispatching to prevent redundant requests - cb.loadingChunks[i] = true - // Dispatch async read chunk task - cb.workerPool.DispatchNonBlocking( - pool.NewReadChunkTask(cb.filename, i, cb.FS), - ) - } else { - // Fallback: synchronous load (should only happen during testing) - _, err := cb.loadChunk(i) - if err != nil { - fmt.Printf("Error prefetching chunk %d: %v\n", i, err) - } - } - } - } - } cb.lastPrefetchedChunk = centerChunk } -// EvictFarChunks removes chunks that are too far from the cursor. -// radius is the number of chunks to keep around the cursor. -// Dirty chunks (modified since the last disk write) are NEVER evicted, -// even if they are far from the cursor, to prevent data loss. +// EvictFarChunks is intentionally a no-op for in-range files: all chunks stay +// resident so byte->chunk mapping and FullContent remain exact. (Evicting +// would reintroduce the stale-disk re-read hazard the fixed-slot model had.) func (cb *ChunkedBuffer) EvictFarChunks(cursorPos int, radius int) { - if cb.fileLen == 0 { - return - } - cursorChunk := cursorPos / cb.chunkSize - numChunks := int((cb.fileLen + int64(cb.chunkSize) - 1) / int64(cb.chunkSize)) + // no-op +} - for i := range cb.chunks { - // NEVER evict dirty chunks — they contain unsaved modifications - if cb.dirtyChunks[i] { - continue - } - if i < cursorChunk-radius || i > cursorChunk+radius { - // Check if chunk index is within valid range before deleting - if i >= 0 && i < numChunks { - delete(cb.chunks, i) - } - } +// markDirtyChunk flags the buffer and a chunk as modified. +func (cb *ChunkedBuffer) markDirtyChunk(idx int) { + cb.dirty = true + if idx >= 0 { + cb.dirtyChunks[idx] = true } } -// Insert inserts text at a given position. +// Insert inserts text at byte position pos, splicing only the affected chunk. func (cb *ChunkedBuffer) Insert(pos int, text string) { if len(text) == 0 { return } - // Ensure the chunk containing pos is loaded - chunkIdx := pos / cb.chunkSize - // Ensure we don't try to insert beyond the current fileLen if it's not a new file - if pos > int(cb.fileLen) && cb.fileLen > 0 { - pos = int(cb.fileLen) // clamp insertion point to end of file + fileLen := int(cb.fileLen) + if pos > fileLen { + pos = fileLen // clamp to end } if pos < 0 { pos = 0 } - - // Load chunk if it doesn't exist, or if pos is at the very beginning of a non-loaded chunk - if _, ok := cb.chunks[chunkIdx]; !ok { - // If inserting at the start of a chunk, we need to load it. - // If inserting past the end of the file, we might create new chunks. - // For now, assume loadChunk handles cases where pos is beyond current fileLen by reading up to fileLen. - _, err := cb.loadChunk(chunkIdx) // This is blocking and might be problematic - if err != nil { - fmt.Printf("Error loading chunk %d for insert: %v\n", chunkIdx, err) - return - } + idx, local := cb.chunkForPos(pos) + if idx < 0 { + // Empty buffer: create the first chunk. + cb.chunks = append(cb.chunks, []byte(text)) + cb.fileLen = int64(len(text)) + cb.markDirtyChunk(0) + return } - - chunk := cb.chunks[chunkIdx] - offsetInChunk := pos - chunkIdx*cb.chunkSize - - // Ensure offsetInChunk is valid for the loaded chunk - if offsetInChunk > len(chunk) { - // This can happen if we are inserting past the end of the loaded chunk, - // which might be due to fileLen not being updated or insertion into new territory. - // For now, we'll pad the chunk if needed. This needs more robust handling. - padding := make([]byte, offsetInChunk-len(chunk)) - chunk = append(chunk, padding...) - } - - // Insert into the chunk - newChunk := make([]byte, len(chunk)+len(text)) - copy(newChunk, chunk[:offsetInChunk]) - copy(newChunk[offsetInChunk:], text) - copy(newChunk[offsetInChunk+len(text):], chunk[offsetInChunk:]) - cb.chunks[chunkIdx] = newChunk - + chunk := cb.chunks[idx] + newChunk := make([]byte, 0, len(chunk)+len(text)) + newChunk = append(newChunk, chunk[:local]...) + newChunk = append(newChunk, text...) + newChunk = append(newChunk, chunk[local:]...) + cb.chunks[idx] = newChunk cb.fileLen += int64(len(text)) - - cb.dirty = true - cb.dirtyChunks[chunkIdx] = true - - // If insertion spans chunk boundary, it might require merging chunks. - // This is complex and might involve resizing subsequent chunks and potentially - // re-reading them. For now, we defer complex merge logic. - // The plan mentions maybeMergeChunk, which would handle this. - // cb.maybeMergeChunk(chunkIdx) + cb.markDirtyChunk(idx) } -// Delete deletes n bytes starting at pos. +// Delete deletes n bytes starting at pos, splicing only the affected chunk(s). func (cb *ChunkedBuffer) Delete(pos, n int) { if n <= 0 { return } - // Clamp pos and n to valid range + fileLen := int(cb.fileLen) if pos < 0 { pos = 0 } - if pos >= int(cb.fileLen) { - return // Nothing to delete + if pos >= fileLen { + return } - if pos+n > int(cb.fileLen) { - n = int(cb.fileLen) - pos + if pos+n > fileLen { + n = fileLen - pos } - - startChunk := pos / cb.chunkSize - endChunk := (pos + n - 1) / cb.chunkSize - - // Load all affected chunks - for i := startChunk; i <= endChunk; i++ { - if i < 0 { - continue - } - if _, ok := cb.chunks[i]; !ok { - _, err := cb.loadChunk(i) - if err != nil { - fmt.Printf("Error loading chunk %d for delete: %v\n", i, err) - return // Abort delete on error - } - } - } - - // 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 { - continue - } + offset := 0 + for i := range cb.chunks { chunk := cb.chunks[i] - if chunk == nil { + chunkStart := offset + chunkEnd := offset + len(chunk) + offset = chunkEnd + if chunkEnd <= pos || chunkStart >= absEnd { + continue // no overlap with [pos, absEnd) + } + delStart := max(pos, chunkStart) - chunkStart + delEnd := min(absEnd, chunkEnd) - chunkStart + if delStart >= delEnd { continue } - - currentChunkStart := i * cb.chunkSize - currentChunkEnd := currentChunkStart + len(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[:localStart], chunk[localEnd:]...) - - remaining -= delEnd - delStart - - // Mark this chunk as dirty so it won't be evicted - cb.dirtyChunks[i] = true + cb.chunks[i] = append(chunk[:delStart], chunk[delEnd:]...) + cb.markDirtyChunk(i) } - - // Update file length cb.fileLen -= int64(n) if cb.fileLen < 0 { cb.fileLen = 0 } - cb.dirty = true } - -// VisibleByteRange returns the byte range [start, end) of content -// visible in the viewport, given the current scroll offset and viewport height. +// VisibleByteRange returns the byte range [start, end) that is visible on the +// current editor viewport, plus the visual line at the top of the viewport. +// This drives the renderer's virtual scrolling: only this range of text is +// laid out into GlyphLayout, keeping memory and layout cost bounded by the +// viewport, not the file. It uses actual (prefix-sum) chunk offsets so the +// range stays correct after length-changing edits. func (cb *ChunkedBuffer) VisibleByteRange(scrollOffset ui.Dp, byteOffset int, viewportHeight ui.Dp, lineHeight ui.Dp, wordWrap bool, layout ui.GlyphLayout, visualIndex *types.VisualLineIndex) (start, end, startLine int) { // If wrapping and we have visual line data, use it for precise calculation if wordWrap && len(layout.VisualLineStarts) > 0 { @@ -491,19 +397,18 @@ func (cb *ChunkedBuffer) VisibleByteRange(scrollOffset ui.Dp, byteOffset int, vi if visualLine >= len(layout.VisualLineStarts) { visualLine = len(layout.VisualLineStarts) - 1 } - + start = layout.VisualLineStarts[visualLine] + byteOffset - fmt.Printf("VisibleByteRange: visualLine = %d start = %d\n", visualLine, start) - + // Calculate end: enough content to fill viewport + buffer - linesInViewport := int(viewportHeight / lineHeight) + 2 + linesInViewport := int(viewportHeight/lineHeight) + 2 endLine := visualLine + linesInViewport if endLine >= len(layout.VisualLineStarts) { end = int(cb.fileLen) } else { end = layout.VisualLineStarts[endLine] } - + // Clamp to file bounds if end > int(cb.fileLen) { end = int(cb.fileLen) @@ -516,7 +421,7 @@ func (cb *ChunkedBuffer) VisibleByteRange(scrollOffset ui.Dp, byteOffset int, vi } return start, end, visualLine } - + // Fallback to LineIndex (logical lines) or estimate if: // 1. Not word wrapping // 2. No visual index available @@ -531,11 +436,6 @@ func (cb *ChunkedBuffer) VisibleByteRange(scrollOffset ui.Dp, byteOffset int, vi // visibleByteRangeEstimate approximates the visible byte range using // heuristic estimates. Used when the line index is not yet available. func (cb *ChunkedBuffer) visibleByteRangeEstimate(scrollOffset ui.Dp, viewportHeight ui.Dp) (start, end int) { - // This is a rough estimation. A proper implementation would need actual line height. - // For simplicity, assuming a fixed line height based on FontSize. - // This requires access to theme or font metrics, which is not directly available here. - // As a fallback, let's use a simplified calculation based on estimated lines and average bytes per line. - // Use the editor's line height constant for consistency. lineHeight := EditorLineHeight() @@ -545,38 +445,40 @@ func (cb *ChunkedBuffer) visibleByteRangeEstimate(scrollOffset ui.Dp, viewportHe // Clamp line numbers to reasonable bounds totalLinesEstimate := 0 if cb.fileLen > 0 { - totalLinesEstimate = int(cb.fileLen / 50) + 1 // Rough estimate: 50 bytes per line + totalLinesEstimate = int(cb.fileLen/50) + 1 // Rough estimate: 50 bytes per line + } + if startLine < 0 { + startLine = 0 + } + if endLine > totalLinesEstimate { + endLine = totalLinesEstimate + } + if startLine >= endLine { + endLine = startLine + 1 // Ensure at least one line is visible } - if startLine < 0 { startLine = 0 } - if endLine > totalLinesEstimate { endLine = totalLinesEstimate } - if startLine >= endLine { endLine = startLine + 1 } // Ensure at least one line is visible - // Convert line numbers to byte offsets using the line index if available - // If LineIndex is nil, we fall back to a very rough byte estimation. + // Convert line numbers to byte offsets using the line index if available. if cb.LineIndex != nil { - // Use LineIndex if it exists if startLine < len(cb.LineIndex.Offsets) { start = int(cb.LineIndex.Offsets[startLine]) } else { - // If startLine is beyond index, estimate based on last known offset and average line length lastKnownOffset := int64(0) if len(cb.LineIndex.Offsets) > 0 { lastKnownOffset = int64(cb.LineIndex.Offsets[len(cb.LineIndex.Offsets)-1]) } - linesBeyondIndex := startLine - (len(cb.LineIndex.Offsets) -1) - start = int(lastKnownOffset + int64(linesBeyondIndex) * 50) // Estimate + linesBeyondIndex := startLine - (len(cb.LineIndex.Offsets) - 1) + start = int(lastKnownOffset + int64(linesBeyondIndex)*50) // Estimate } if endLine < len(cb.LineIndex.Offsets) { end = int(cb.LineIndex.Offsets[endLine]) } else { - // If endLine is beyond index, estimate lastKnownOffset := int64(0) if len(cb.LineIndex.Offsets) > 0 { lastKnownOffset = int64(cb.LineIndex.Offsets[len(cb.LineIndex.Offsets)-1]) } - linesBeyondIndex := endLine - (len(cb.LineIndex.Offsets) -1) - end = int(lastKnownOffset + int64(linesBeyondIndex) * 50) // Estimate + linesBeyondIndex := endLine - (len(cb.LineIndex.Offsets) - 1) + end = int(lastKnownOffset + int64(linesBeyondIndex)*50) // Estimate } } else { // Rough byte estimation if no LineIndex @@ -586,9 +488,15 @@ func (cb *ChunkedBuffer) visibleByteRangeEstimate(scrollOffset ui.Dp, viewportHe // Clamp to file bounds if cb.fileLen > 0 { - if start < 0 { start = 0 } - if end > int(cb.fileLen) { end = int(cb.fileLen) } - if end <= start { end = start + cb.chunkSize } // Ensure at least one chunk's worth if range is invalid + if start < 0 { + start = 0 + } + if end > int(cb.fileLen) { + end = int(cb.fileLen) + } + if end <= start { + end = start + cb.chunkSize // Ensure at least one chunk's worth if range is invalid + } } else { start = 0 end = 0 // Empty file @@ -600,79 +508,64 @@ func (cb *ChunkedBuffer) visibleByteRangeEstimate(scrollOffset ui.Dp, viewportHe // visibleByteRangePrecise uses the LineIndex to find the exact byte range. func (cb *ChunkedBuffer) visibleByteRangePrecise(scrollOffset ui.Dp, viewportHeight ui.Dp) (start, end int) { if cb.LineIndex == nil || len(cb.LineIndex.Offsets) == 0 { - // Should not happen if called after LineIndex is available, but as a safeguard: return cb.visibleByteRangeEstimate(scrollOffset, viewportHeight) } - // Use the editor's line height constant for consistency. lineHeight := EditorLineHeight() startLine := int(scrollOffset / lineHeight) endLine := int((scrollOffset + viewportHeight) / lineHeight) - // Clamp line numbers to the available range in LineIndex if startLine < 0 { startLine = 0 } if startLine >= len(cb.LineIndex.Offsets) { - startLine = len(cb.LineIndex.Offsets) - 1 // Last available line + startLine = len(cb.LineIndex.Offsets) - 1 } - - if endLine < 0 { // Should not happen with positive viewportHeight + if endLine < 0 { endLine = 0 } if endLine >= len(cb.LineIndex.Offsets) { - endLine = len(cb.LineIndex.Offsets) - 1 // Last available line + endLine = len(cb.LineIndex.Offsets) - 1 } - // Ensure endLine is at least startLine + 1, unless startLine is already the last line. if startLine < len(cb.LineIndex.Offsets)-1 && endLine <= startLine { endLine = startLine + 1 } - start = int(cb.LineIndex.Offsets[startLine]) - // The end byte offset is the start of the *next* line after the visible range. - // If endLine is the last line in the index, the end byte offset is the file length. if endLine+1 < len(cb.LineIndex.Offsets) { end = int(cb.LineIndex.Offsets[endLine+1]) } else { - end = int(cb.fileLen) // Use file length as the end if it's the last line + end = int(cb.fileLen) } - // Ensure range is within file bounds - if start < 0 { start = 0 } - if end > int(cb.fileLen) { end = int(cb.fileLen) } + if start < 0 { + start = 0 + } + if end > int(cb.fileLen) { + end = int(cb.fileLen) + } if end <= start { - // If somehow the range is invalid, return a minimal valid range, - // e.g., start of the start line to a bit past it, or just end of file. if start < int(cb.fileLen) { - end = min(start+cb.chunkSize, int(cb.fileLen)) // At least one chunk or up to file end + end = min(start+cb.chunkSize, int(cb.fileLen)) } else { - end = start // If start is already at file end, range is empty + end = start } } return start, end } -// maybeMergeChunk is a placeholder for logic that consolidates chunks if they become too small -// or if edits cause fragmentation. This is complex and deferred. -func (cb *ChunkedBuffer) maybeMergeChunk(chunkIdx int) { - // Placeholder for future implementation - // This would involve checking chunk sizes and potentially merging adjacent chunks. -} - -// UpdateLineIndexAfterEdit updates the LineIndex offsets after an insert or delete. -// offsetShift is the number of bytes inserted (positive) or deleted (negative). -// editPos is the byte position where the edit occurred. -// This is a partial update: only offsets after editPos are shifted. +// UpdateLineIndexAfterEdit updates the LineIndex offsets after an insert or +// delete. offsetShift is the number of bytes inserted (positive) or deleted +// (negative); editPos is the byte position where the edit occurred. func (cb *ChunkedBuffer) UpdateLineIndexAfterEdit(editPos int, offsetShift int) { if cb.LineIndex == nil { return } - // Find the first offset that needs updating using binary search. - // All offsets >= editPos need to be shifted by offsetShift. + // Find the first offset that needs updating using binary search. All + // offsets >= editPos are shifted by offsetShift. idx := sort.Search(len(cb.LineIndex.Offsets), func(i int) bool { return int(cb.LineIndex.Offsets[i]) >= editPos }) @@ -682,14 +575,13 @@ func (cb *ChunkedBuffer) UpdateLineIndexAfterEdit(editPos int, offsetShift int) for i := idx; i < len(cb.LineIndex.Offsets); i++ { cb.LineIndex.Offsets[i] += int32(offsetShift) } - // Update the file size stamp cb.LineIndex.Size += int64(offsetShift) if cb.LineIndex.Size < 0 { cb.LineIndex.Size = 0 } } -// Helper function for max +// max returns the larger of two ints. func max(a, b int) int { if a > b { return a @@ -697,12 +589,10 @@ func max(a, b int) int { return b } -// Helper function for min +// min returns the smaller of two ints. func min(a, b int) int { if a < b { return a } return b } - - diff --git a/internal/editor/chunked_buffer_test.go b/internal/editor/chunked_buffer_test.go index f8b7ff6..c1fe231 100644 --- a/internal/editor/chunked_buffer_test.go +++ b/internal/editor/chunked_buffer_test.go @@ -1,176 +1,193 @@ package editor import ( + "bytes" + "strings" "testing" - "time" - - "pad/internal/io/pool/mock" ) -// TestEvictDirtyChunk_LosesEdits verifies that editing a chunk and then -// evicting it does NOT lose the modification. Dirty chunks are protected -// from eviction even when far from the cursor. -func TestEvictDirtyChunk_LosesEdits(t *testing.T) { - mockFS := mock.NewFileSystem() - filename := "/test.txt" +// newTestBuffer builds a fully-loaded (in-range) buffer from content using the +// on-open path (SetContent), so all chunks are resident. +func newTestBuffer(t *testing.T, content []byte) *ChunkedBuffer { + t.Helper() + cb := NewChunkedBuffer("/test.txt", DefaultChunkSize, nil, "") + cb.SetContent(content) + return cb +} - // Create a 200KB file (3 chunks: 64KB, 64KB, 72KB) +// TestSetContent_MultipleChunks verifies the whole-file load splits content +// into ordered chunks and Content reconstructs it exactly. +func TestSetContent_MultipleChunks(t *testing.T) { content := make([]byte, 200*1024) for i := range content { - content[i] = byte(i % 256) + content[i] = byte(i % 251) } - mockFS.AddFile(filename, content, time.Now()) + cb := newTestBuffer(t, content) - cb := NewChunkedBuffer(filename, DefaultChunkSize, mockFS, "") - cb.SetFileSize(200 * 1024) - cb.LoadChunk(0) - - // 1. Insert 100 bytes at position 0, modifying chunk 0 - insertText := "MODIFIED" - cb.Insert(0, insertText) - - // Verify the chunk grew - if len(cb.chunks[0]) != DefaultChunkSize+len(insertText) { - t.Fatalf("expected chunk 0 to grow to %d, got %d", DefaultChunkSize+len(insertText), len(cb.chunks[0])) + // 200KB / 64KB = 3 full chunks (64KB each) + 1 partial (8KB) = 4 chunks. + if got := len(cb.chunks); got != 4 { + t.Fatalf("expected 4 chunks, got %d", got) } - - // 2. Simulate scrolling to chunk 2, which would normally trigger eviction of chunk 0 - cb.EvictFarChunks(2*DefaultChunkSize, 1) - - // Chunk 0 should NOT be evicted because it's dirty - if _, ok := cb.chunks[0]; !ok { - t.Fatal("expected dirty chunk 0 to survive eviction") + if cb.FileLen() != int64(len(content)) { + t.Fatalf("FileLen=%d, want %d", cb.FileLen(), len(content)) } - - // 3. Reconstruct the full file — the edit at position 0 must survive full, err := cb.FullContent() if err != nil { - t.Fatalf("unexpected error: %v", err) + t.Fatalf("FullContent error: %v", err) } - - // The first len(insertText) bytes must be the inserted text - if full[:len(insertText)] != insertText { - t.Errorf("expected first %d bytes to be %q, got %q", len(insertText), insertText, full[:len(insertText)]) + if full != string(content) { + t.Fatalf("FullContent mismatch after SetContent") + } + // Spot-check a range that spans the chunk-0/chunk-1 boundary. + seg := cb.Content(64*1024-10, 64*1024+10) + if seg != string(content[64*1024-10:64*1024+10]) { + t.Fatalf("Content across chunk boundary mismatch: got %q", seg) } } -// TestEvictDirtyChunk_DeleteLosesEdits verifies that a deletion in a chunk -// survives eviction. Dirty chunks are never evicted. -func TestEvictDirtyChunk_DeleteLosesEdits(t *testing.T) { - mockFS := mock.NewFileSystem() - filename := "/test.txt" - - // Create a 128KB file (2 chunks of 64KB) - content := make([]byte, 128*1024) - for i := range content { - content[i] = byte(i % 256) - } - mockFS.AddFile(filename, content, time.Now()) - - cb := NewChunkedBuffer(filename, DefaultChunkSize, mockFS, "") - cb.SetFileSize(128 * 1024) - cb.LoadChunk(0) - cb.LoadChunk(1) - - // 1. Delete 100 bytes at position 127000 (in chunk 1) - cb.Delete(127000, 100) - - // Verify chunk 1 shrunk - if len(cb.chunks[1]) != DefaultChunkSize-100 { - t.Fatalf("expected chunk 1 to shrink to %d, got %d", DefaultChunkSize-100, len(cb.chunks[1])) - } - - // 2. Evict — chunk 1 should NOT be evicted because it's dirty - cb.EvictFarChunks(0, 1) - - if _, ok := cb.chunks[1]; !ok { - t.Fatal("expected dirty chunk 1 to survive eviction") - } - - // 3. Reconstruct — the deletion must survive - full, err := cb.FullContent() - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - expectedLen := 128*1024 - 100 - if len(full) != expectedLen { - t.Errorf("expected file length %d after deletion, got %d", expectedLen, len(full)) - } -} - -// TestEvictCleanChunk_AllowsEviction verifies that chunks WITHOUT edits -// CAN be evicted (i.e., dirty-chunk tracking doesn't prevent normal eviction). -func TestEvictCleanChunk_AllowsEviction(t *testing.T) { - mockFS := mock.NewFileSystem() - filename := "/test.txt" - +// TestInsertShiftsLaterChunks is the core drift-fix test: an insert in chunk 0 +// must shift every later byte, and Content must still return the correct byte +// at the shifted offset (the old fixed-slot i*chunkSize model failed here). +func TestInsertShiftsLaterChunks(t *testing.T) { content := make([]byte, 200*1024) for i := range content { - content[i] = byte(i % 256) + content[i] = byte(i % 251) } - mockFS.AddFile(filename, content, time.Now()) + cb := newTestBuffer(t, content) - cb := NewChunkedBuffer(filename, DefaultChunkSize, mockFS, "") - cb.SetFileSize(200 * 1024) - cb.LoadChunk(0) - cb.LoadChunk(1) - cb.LoadChunk(2) + const insLen = 3 + cb.Insert(0, strings.Repeat("X", insLen)) - // No edits — all chunks are clean - // Evict chunk 0 (far from cursor at chunk 2) - t.Logf("DEBUG: chunks before eviction: %v", cb.chunks) - cb.EvictFarChunks(2*DefaultChunkSize, 1) - t.Logf("DEBUG: chunks after eviction: %v", cb.chunks) - - // Chunk 0 should be evicted (it's clean) - if _, evicted := cb.chunks[0]; evicted { - t.Fatalf("expected clean chunk 0 to be evicted, but it still exists (chunks=%v)", cb.chunks) + // A byte originally at position 100000 (chunk 1) is now at 100003. + want := content[100000] + if got := cb.Content(100000+insLen, 100001+insLen); got != string(want) { + t.Fatalf("byte at shifted offset 100003 = %q, want %q", got, string(want)) } - - // But chunks 1 and 2 should remain - if _, ok := cb.chunks[1]; !ok { - t.Fatal("expected chunk 1 to remain") + // The inserted prefix is present. + if got := cb.Content(0, insLen); got != strings.Repeat("X", insLen) { + t.Fatalf("prefix = %q, want %q", got, strings.Repeat("X", insLen)) } - if _, ok := cb.chunks[2]; !ok { - t.Fatal("expected chunk 2 to remain") + // Full content is the insert + original. + full, _ := cb.FullContent() + if full != strings.Repeat("X", insLen)+string(content) { + t.Fatalf("FullContent after insert mismatch (len=%d, want %d)", len(full), insLen+len(content)) } } -// TestFullContent_AfterEvictDirtyChunk_ReReadsFromDisk verifies that when a dirty -// chunk is somehow evicted (e.g., by a bug), FullContent re-reads the OLD disk -// content — confirming the data loss scenario. This test documents the bug before -// it is fixed. -func TestFullContent_AfterEvictDirtyChunk_ReReadsFromDisk(t *testing.T) { - mockFS := mock.NewFileSystem() - filename := "/test.txt" - - // Create a small 100-byte file (1 chunk) - content := []byte("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA") - mockFS.AddFile(filename, content, time.Now()) - - cb := NewChunkedBuffer(filename, DefaultChunkSize, mockFS, "") - cb.SetFileSize(100) - cb.LoadChunk(0) - - // Insert at position 0 — modifies chunk 0 in memory - cb.Insert(0, "X") - - // The in-memory chunk 0 should now be 101 bytes starting with "X" - t.Logf("DEBUG: chunk 0 len=%d, first byte=%q, dirtyChunks=%v", len(cb.chunks[0]), cb.chunks[0][0], cb.dirtyChunks) - if cb.chunks[0][0] != 'X' { - t.Fatal("expected in-memory chunk to start with 'X'") +// TestDeleteShiftsLaterChunks verifies a deletion in an early chunk shifts +// later bytes left and Content/FullContent stay exact. +func TestDeleteShiftsLaterChunks(t *testing.T) { + content := make([]byte, 200*1024) + for i := range content { + content[i] = byte(i % 251) } + cb := newTestBuffer(t, content) - // Manually evict the dirty chunk (simulating the bug) - t.Logf("DEBUG: before delete, chunks=%v", cb.chunks) - delete(cb.chunks, 0) - t.Logf("DEBUG: after delete, chunks=%v", cb.chunks) + const delLen = 100 + cb.Delete(0, delLen) - // FullContent will now return an error because the dirty chunk is missing - full, err := cb.FullContent() - if err == nil { - t.Errorf("expected error due to missing dirty chunk, got nil, full content: %q", full) - } else { - t.Logf("DEBUG: caught expected error: %v", err) + // A byte originally at 100000 is now at 99900. + want := content[100000] + if got := cb.Content(100000-delLen, 100001-delLen); got != string(want) { + t.Fatalf("byte at shifted offset = %q, want %q", got, string(want)) + } + full, _ := cb.FullContent() + if full != string(content[delLen:]) { + t.Fatalf("FullContent after delete mismatch (len=%d, want %d)", len(full), len(content)-delLen) + } +} + +// TestDeleteSpanningChunks verifies a deletion that spans multiple chunk +// boundaries is exact. +func TestDeleteSpanningChunks(t *testing.T) { + content := make([]byte, 200*1024) + for i := range content { + content[i] = byte(i % 251) + } + cb := newTestBuffer(t, content) + + // Delete 64KB+50 bytes starting at 30KB (spans chunks 0,1,2). + const start = 30 * 1024 + const n = 64*1024 + 50 + cb.Delete(start, n) + + expected := make([]byte, 0, len(content)-n) + expected = append(expected, content[:start]...) + expected = append(expected, content[start+n:]...) + full, _ := cb.FullContent() + if !bytes.Equal([]byte(full), expected) { + t.Fatalf("FullContent after spanning delete mismatch (len=%d, want %d)", len(full), len(expected)) + } + if cb.FileLen() != int64(len(expected)) { + t.Fatalf("FileLen=%d, want %d", cb.FileLen(), len(expected)) + } +} + +// TestRuneIndexToByteAfterEdit verifies the IME rune->byte bridge stays correct +// after a length-changing edit (ASCII + multibyte). +func TestRuneIndexToByteAfterEdit(t *testing.T) { + // 2 chunks of ASCII so the edit crosses into chunk arithmetic. + var b bytes.Buffer + for i := 0; i < 100*1024; i++ { + b.WriteByte(byte('a' + i%26)) + } + cb := newTestBuffer(t, b.Bytes()) + + // Insert a multibyte rune ("é" = 2 bytes) at rune 0. + cb.Insert(0, "é") + // Rune 0 is now 'é' (bytes 0..1). Rune 1 is the original first 'a' at byte 2. + if got := cb.RuneIndexToByte(0); got != 0 { + t.Fatalf("RuneIndexToByte(0)=%d, want 0", got) + } + if got := cb.RuneIndexToByte(1); got != 2 { + t.Fatalf("RuneIndexToByte(1)=%d, want 2", got) + } + // New rune R (R>0) is the original rune R-1 (originally at byte R-1), now + // at byte (R-1)+2 = R+1 (é is 1 rune, 2 bytes). Check one deep in chunk 1 + // (past the 64KB boundary) to confirm the shift survives chunk boundaries. + deep := 70000 + if got := cb.RuneIndexToByte(deep); got != deep+1 { + t.Fatalf("RuneIndexToByte(%d)=%d, want %d", deep, got, deep+1) + } +} + +// TestContent_BoundsAndEmpty guards edge cases in the rewritten Content. +func TestContent_BoundsAndEmpty(t *testing.T) { + cb := newTestBuffer(t, []byte("hello world")) + if got := cb.Content(0, 5); got != "hello" { + t.Fatalf("Content(0,5)=%q, want hello", got) + } + if got := cb.Content(6, 11); got != "world" { + t.Fatalf("Content(6,11)=%q, want world", got) + } + // Out-of-range end clamps to fileLen. + if got := cb.Content(0, 1000); got != "hello world" { + t.Fatalf("Content(0,1000)=%q, want full", got) + } + // start >= end is empty. + if got := cb.Content(5, 5); got != "" { + t.Fatalf("Content(5,5)=%q, want empty", got) + } + // Empty buffer. + empty := newTestBuffer(t, nil) + if got := empty.Content(0, 10); got != "" { + t.Fatalf("empty Content=%q, want empty", got) + } + if got, _ := empty.FullContent(); got != "" { + t.Fatalf("empty FullContent=%q, want empty", got) + } +} + +// TestAppendAtEnd inserts at the very end of the buffer (pos == fileLen), which +// maps to the last chunk's end. +func TestAppendAtEnd(t *testing.T) { + cb := newTestBuffer(t, []byte("abcdef")) + cb.Insert(6, "XYZ") + full, _ := cb.FullContent() + if full != "abcdefXYZ" { + t.Fatalf("append at end = %q, want abcdefXYZ", full) + } + if cb.FileLen() != 9 { + t.Fatalf("FileLen=%d, want 9", cb.FileLen()) } } diff --git a/internal/editor/e2e_test.go b/internal/editor/e2e_test.go index 14d9019..a4cd483 100644 --- a/internal/editor/e2e_test.go +++ b/internal/editor/e2e_test.go @@ -184,45 +184,16 @@ func TestLargeFileChunkBoundary(t *testing.T) { t.Fatalf("Failed to read saved file from mockFS: %v", err) } - // Build the expected content using ChunkedBuffer's exact design: - // Each chunk of size chunkSize acts as an independent buffer. - // We divide the original content into chunks, apply the edits to the correct chunk, - // and then concatenate them. - chunks := make([][]byte, 4) - for i := 0; i < 4; i++ { - start := i * chunkSize - end := start + chunkSize - if end > fileSize { - end = fileSize - } - chunks[i] = append([]byte(nil), initialContent[start:end]...) - } - - // Helper to insert into a specific chunk - insertInChunk := func(chunkIdx, offset int, txt string) { - chunk := chunks[chunkIdx] - newChunk := make([]byte, len(chunk)+len(txt)) - copy(newChunk, chunk[:offset]) - copy(newChunk[offset:], txt) - copy(newChunk[offset+len(txt):], chunk[offset:]) - chunks[chunkIdx] = newChunk - } - - // 1. "CHUNK1" (pos 65000 -> chunk 0, offset 65000) - insertInChunk(0, 65000, "CHUNK1") - - // 2. "BOUNDARY" (pos 65535 -> chunk 0, offset 65535) - insertInChunk(0, 65535, "BOUNDARY") - - // 3. "CHUNK3" (pos 262000 -> chunk 3, offset 262000 - 3*chunkSize = 65392) - insertInChunk(3, 262000-3*chunkSize, "CHUNK3") - - // Concatenate chunks to get expected content - var expectedBuf bytes.Buffer - for _, chunk := range chunks { - expectedBuf.Write(chunk) - } - expected := expectedBuf.Bytes() + // Build the expected content by applying the same three inserts to the + // original content as a whole, in order. This is the shift-correct ground + // truth: each insert at an absolute position pushes all later bytes right. + // (The old fixed-slot model treated each chunk as an independent buffer and + // did NOT shift later chunks; that was the bug this design replaces.) + expectedStr := string(initialContent) + expectedStr = expectedStr[:65000] + "CHUNK1" + expectedStr[65000:] // insert 1 @65000 + expectedStr = expectedStr[:65535] + "BOUNDARY" + expectedStr[65535:] // insert 2 @65535 + expectedStr = expectedStr[:262000] + "CHUNK3" + expectedStr[262000:] // insert 3 @262000 + expected := []byte(expectedStr) if len(savedContent) != len(expected) { t.Errorf("Saved file length %d != expected %d (too long by %d, too short by %d)", diff --git a/internal/editor/ime_range_test.go b/internal/editor/ime_range_test.go index e2157df..a75144a 100644 --- a/internal/editor/ime_range_test.go +++ b/internal/editor/ime_range_test.go @@ -27,11 +27,8 @@ func newChunkedState(t *testing.T, content string, chunkSize int) *State { 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) - } + // Full-load via the on-open path (all chunks resident), matching production. + cb.SetContent([]byte(content)) st := NewState() TheState = st st.Editor.Filename = filename diff --git a/internal/editor/logic.go b/internal/editor/logic.go index f308fd3..4beeb18 100644 --- a/internal/editor/logic.go +++ b/internal/editor/logic.go @@ -291,9 +291,10 @@ func (l *Logic) handleWorkerResult(res pool.Result) { if res.Success { if content, ok := res.Data.([]byte); ok { if l.state.Editor.ChunkedBuffer != nil { - // Always populate the chunked buffer and ensure its size is set. + // Full-load the in-range file: set the whole content (split into + // resident chunks). No lazy load, so no stale-disk re-read. l.state.Editor.ChunkedBuffer.SetFileSize(int64(len(content))) - l.state.Editor.ChunkedBuffer.Insert(0, string(content)) + l.state.Editor.ChunkedBuffer.SetContent(content) // Also update the Buffer field for backward compatibility and for tests checking it. l.state.Editor.Buffer = string(content) } else { @@ -303,11 +304,15 @@ func (l *Logic) handleWorkerResult(res pool.Result) { } } } else if res.TaskType == pool.TypeReadChunk { + // In-range files load fully via SetContent, so this is only a fallback. if cb := l.state.Editor.ChunkedBuffer; cb != nil { delete(cb.loadingChunks, res.ChunkIdx) // Use explicit ChunkIdx field if res.Success { if chunk, ok := res.Data.([]byte); ok { + for len(cb.chunks) <= res.ChunkIdx { + cb.chunks = append(cb.chunks, nil) + } cb.chunks[res.ChunkIdx] = chunk log.Printf("Logic: Loaded chunk %d for %s (%d bytes)", res.ChunkIdx, res.FilePath, len(chunk)) } @@ -319,14 +324,20 @@ func (l *Logic) handleWorkerResult(res pool.Result) { log.Printf("Logic: TypeStatFile result success=%v", res.Success) if res.Success { if stat, ok := res.Data.(*pool.FileStat); ok { + // Size guard: refuse to edit files above the limit. The browser can + // still list them; the editor shows a "too large to edit" notice. + if stat.Size > MaxEditableFileSize { + l.state.Editor.TooLarge = true + l.state.Editor.TooLargeSize = stat.Size + log.Printf("Logic: %s is %d bytes, exceeds the %d-byte edit limit", stat.Path, stat.Size, MaxEditableFileSize) + l.frameChan <- l.frameOf(l.state.layout(l.browserManager)) + return + } if l.state.Editor.ChunkedBuffer != nil { l.state.Editor.ChunkedBuffer.SetFileSize(stat.Size) - // Load the chunk containing the cursor (position 0 at open) - l.state.Editor.ChunkedBuffer.LoadChunk(0) - // Prefetch adjacent chunks - l.state.Editor.ChunkedBuffer.Prefetch(0, 1) - // Dispatch line index build task - l.workerPool.Dispatch(pool.NewBuildLineIndexTask(l.state.Editor.Filename, l.mockFS)) + // Full-load: read the whole file and dispatch a line index build. + l.workerPool.Dispatch(pool.NewReadFileTask(stat.Path, l.mockFS)) + l.workerPool.Dispatch(pool.NewBuildLineIndexTask(stat.Path, l.mockFS)) } } } diff --git a/internal/editor/state.go b/internal/editor/state.go index 695c766..5b0515a 100644 --- a/internal/editor/state.go +++ b/internal/editor/state.go @@ -25,6 +25,20 @@ func EditorLineHeight() ui.Dp { return ui.Dp(float32(EditorFontSize) * EditorLineHeightScale) } +// formatSize renders a byte count as a human-readable size (e.g. "10.4 MB"). +func formatSize(n int64) string { + const unit = 1024 + if n < unit { + return fmt.Sprintf("%d B", n) + } + div, exp := int64(unit), 0 + for m := n / unit; m >= unit; m /= unit { + div *= unit + exp++ + } + return fmt.Sprintf("%.1f %cB", float64(n)/float64(div), "kMG"[exp]) +} + // Page identifies which page the app is showing. type Page int @@ -63,7 +77,12 @@ type EditorState struct { // It is set during layout and is what an EditEvent.Range indexes into. IMEWindowText string Filename string - fileVersion map[string]int + // TooLarge is set when an opened file exceeds MaxEditableFileSize. The + // editor shows a "too large to edit" notice instead of content (the + // browser can still list the file). + TooLarge bool + TooLargeSize int64 + fileVersion map[string]int lastWriteVersion map[string]int saveTimer *time.Timer writeFailed map[string]bool // Added: tracks failed writes for UI @@ -128,6 +147,7 @@ type State struct { FocusedElementID string // ID of the currently focused element Elems []ui.Element lastEvictionTime time.Time // Throttles chunk eviction + justOpenedAt time.Time // when the editor page was last opened; used to swallow the opening tap // Browser state (directly embedded per architecture §8) Browser browser.BrowserState // Embedded, not a pointer // Editor state @@ -208,6 +228,12 @@ func ToggleWordWrap(data any) { // Clamped to [0, MaxScroll] so content doesn't scroll past its ends. // Also evicts chunks far from the cursor to keep memory bounded. func HandleScroll(data any) { + // Swallow scroll from the opening gesture: the tap that opened the file can + // leak a scroll delta into the editor before the line index is built (when + // MaxScroll is a large estimate), leaving the viewport past the content. + if time.Since(TheState.justOpenedAt) < 300*time.Millisecond { + return + } delta := data.(int) // pixels TheState.ScrollOffset += ui.ToDp(ui.Px(delta), TheState.scale) if TheState.ScrollOffset < 0 { @@ -266,9 +292,16 @@ func OpenFile(data any) { TheState.Editor.Filename = filename TheState.Editor.CursorPosition = 0 // Reset cursor to top + TheState.Editor.TooLarge = false + TheState.Editor.TooLargeSize = 0 TheState.ScrollOffset = 0 // Reset editor scroll to top when opening a new file TheState.page = EditorPage TheState.FocusedElementID = "editor_text" // Set focus to editor + // The tap that opened this file is delivered to the browser row, but its + // gesture can leak into the now-visible editor and move the cursor/scroll. + // Record the open time so the editor can swallow taps in a short window + // right after open (the opening tap, not a deliberate editor tap). + TheState.justOpenedAt = time.Now() } // SetChunkedBuffer sets the chunked buffer for the current editor state. @@ -320,6 +353,10 @@ func HandleCursorMove(delta int) { // HandleKeyDown interprets keyboard events for navigation and editing. // Receives both key.Event (as key.Name) and key.EditEvent from the main loop. func HandleKeyDown(data any) { + // A too-large file is not editable: ignore all key input. + if TheState.Editor.TooLarge { + return + } log.Printf("HandleKeyDown: received data of type %T: %v", data, data) switch v := data.(type) { case key.EditEvent: @@ -636,6 +673,10 @@ func runeIndexToByteStr(s string, n int) int { // 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) { + // A too-large file is not editable: ignore IME commits. + if TheState.Editor.TooLarge { + return + } if startRune > endRune { startRune, endRune = endRune, startRune } @@ -819,6 +860,16 @@ func EditorLayout(screenWidth, screenHeight ui.Dp, wordWrap bool) []ui.Element { maxScroll = 0 } TheState.MaxScroll = maxScroll + // Keep the viewport within [0, MaxScroll] even when MaxScroll just shrank + // (the line index finished building, or a shorter file was opened). Without + // this, a ScrollOffset set while MaxScroll was the large pre-index estimate + // would stay past the (now smaller) content and render a blank viewport. + if TheState.ScrollOffset < 0 { + TheState.ScrollOffset = 0 + } + if TheState.ScrollOffset > maxScroll { + TheState.ScrollOffset = maxScroll + } // Compute visible content for virtual scrolling. var visibleContent string @@ -827,7 +878,15 @@ func EditorLayout(screenWidth, screenHeight ui.Dp, wordWrap bool) []ui.Element { var start, end int cb := TheState.Editor.ChunkedBuffer - if cb != nil { + if TheState.Editor.TooLarge { + // The file exceeds the edit limit: show a notice instead of content and + // do not edit (the browser can still list the file). + visibleContent = fmt.Sprintf( + "Too large to edit\n\n%s is %s, above the %s limit.\nIt is still listed in the browser.", + TheState.Editor.Filename, formatSize(TheState.Editor.TooLargeSize), formatSize(MaxEditableFileSize)) + visibleCursorPos = 0 + visibleScrollOffset = 0 + } else if cb != nil { viewportHeight := editorRegion.H lineHeight := EditorLineHeight() if lh := TheState.Editor.GlyphLayout.LineHeight; lh > 0 { @@ -844,9 +903,10 @@ func EditorLayout(screenWidth, screenHeight ui.Dp, wordWrap bool) []ui.Element { } } - // Extract visible content from chunked buffer - //visibleContent = cb.Content(start, end) - visibleContent = cb.Content(start, start+2000) + // Extract visible content from chunked buffer. Read the full visible + // range [start, end) so a tall viewport is filled (a fixed 2000-byte + // window could leave the lower rows blank). + visibleContent = cb.Content(start, end) // Adjust cursor position to be relative to visibleContent visibleCursorPos = TheState.Editor.CursorPosition - start @@ -886,13 +946,6 @@ func EditorLayout(screenWidth, screenHeight ui.Dp, wordWrap bool) []ui.Element { visibleScrollOffset = TheState.ScrollOffset } - var s string - if len(visibleContent)>10 { - s = visibleContent[:10] - } else { - s = visibleContent - } - fmt.Printf("LAYOUT: visibleScrollOffset: %f visibleContent (%d) = %s...\n",visibleScrollOffset,len(visibleContent),s) // Record the visible window for IME: the snippet is this window, so an // EditEvent.Range (relative to the window) is offset by IMEWindowStartByte // to address the buffer. start is 0 for small (string) files, so the @@ -911,6 +964,13 @@ func EditorLayout(screenWidth, screenHeight ui.Dp, wordWrap bool) []ui.Element { {Gesture: ui.Scroll, Handler: HandleScroll}, {Gesture: ui.KeyDown, Handler: HandleKeyDown}, {Gesture: ui.Tap, Handler: func(data any) { + // Swallow the tap that opened the file. Its gesture belongs to the + // browser row we just left and must not position the cursor in the + // editor (a deliberate tap-to-position happens later, well past this + // window). + if time.Since(TheState.justOpenedAt) < 300*time.Millisecond { + return + } if pt, ok := data.(ui.Point); ok { // Convert window-space tap coordinates to text-local coordinates. // layout.X is relative to the text region left, and layout.Y is relative to the text region top.