From 93a5f879f556ede0c8a3755044aab85f0b4877ac Mon Sep 17 00:00:00 2001 From: Greg Pomerantz Date: Thu, 4 Jun 2026 18:01:51 -0400 Subject: [PATCH] feat: implement atomic writes for filesystem backend - Add WriteFile method to mock filesystem (non-atomic path, creates files) - Implement WriteFileAtomic in mock with temp file + rename pattern using .tmp/ directory, matching real filesystem semantics - Update WriteFileTask.Execute() to use WriteFileAtomic - Update FlushAll() to use WriteFileAtomic - Fix mock to create files on write (matching os.WriteFile behavior) - Update tests to match new semantics (create-if-not-exists) --- cmd/pad/impl_android.go | 11 +++ cmd/pad/impl_other.go | 7 ++ cmd/pad/main.go | 2 +- doc/editor_implementation_plan.md | 4 +- internal/editor/auto_save_test.go | 7 +- internal/editor/filename_test.go | 54 +++++++++++ internal/editor/logic.go | 2 +- internal/editor/state.go | 117 ++++++++++++++++++++++- internal/io/pool/filesystem.go | 1 + internal/io/pool/mock/filesystem.go | 113 +++++++++++++++++++--- internal/io/pool/mock/filesystem_test.go | 16 +++- internal/io/pool/real/filesystem.go | 9 +- internal/io/pool/task.go | 2 +- internal/io/pool/task_test.go | 17 +++- internal/ui/element.go | 8 +- internal/ui/render.go | 2 +- 16 files changed, 334 insertions(+), 38 deletions(-) create mode 100644 cmd/pad/impl_android.go create mode 100644 cmd/pad/impl_other.go create mode 100644 internal/editor/filename_test.go diff --git a/cmd/pad/impl_android.go b/cmd/pad/impl_android.go new file mode 100644 index 0000000..2cb1713 --- /dev/null +++ b/cmd/pad/impl_android.go @@ -0,0 +1,11 @@ +//+build !darwin !linux + +package main + +import ( + _ "gioui.org/app/permission/storage" +) + +var ( + startpath="/storage/emulated/0/Notes" +) diff --git a/cmd/pad/impl_other.go b/cmd/pad/impl_other.go new file mode 100644 index 0000000..1c90951 --- /dev/null +++ b/cmd/pad/impl_other.go @@ -0,0 +1,7 @@ +//+build !android + +package main + +var ( + startpath="." +) diff --git a/cmd/pad/main.go b/cmd/pad/main.go index aba37cf..2e517d2 100644 --- a/cmd/pad/main.go +++ b/cmd/pad/main.go @@ -38,7 +38,7 @@ func run(w *app.Window) error { shaper := text.NewShaper(text.WithCollection(gofont.Collection())) // Determine root directory for real filesystem - rootDir := flag.String("root", ".", "root directory for the filesystem") + rootDir := flag.String("root", startpath, "root directory for the filesystem") flag.Parse() abs, err := filepath.Abs(*rootDir) diff --git a/doc/editor_implementation_plan.md b/doc/editor_implementation_plan.md index a2994c7..1fd55ea 100644 --- a/doc/editor_implementation_plan.md +++ b/doc/editor_implementation_plan.md @@ -212,11 +212,11 @@ Key design points: ### Phase 2: File IO Integration - [x] Implement `SaveFile` handler: Debounced auto-save via `WriteFileTask` to worker pool. — **DONE** (see §3) - [x] Implement `LoadFile` handler: Dispatch `ReadFileTask` to worker pool. — **DONE** -- [ ] Status bar integration: Show "Saving..." indicator, "Modified" status. — **UPDATED**: Added `` indicator when `WriteFailed()`. status bar also needs "Saving..." indicator. +- [x] Status bar integration: Show "Saving..." indicator, "Modified" status. — **DONE** ### Phase 3: Text Editing Operations - [ ] Implement Cut/Copy/Paste interactions. — **NOT STARTED** (icon elements exist but handlers are `nil`) -- [ ] Implement multi-line text navigation (Home/End, PageUp/PageDown). — **NOT STARTED** +- [x] Implement multi-line text navigation (Home/End, PageUp/PageDown). — **DONE** - [ ] Implement text selection display and mouse interaction. — **NOT STARTED** (`SelectionStart`/`SelectionEnd` fields exist in `EditorState` but are unused) - [x] Implement soft-wrap toggle. — **DONE** (`ToggleWordWrap` in `state.go`, wired to bottom bar "Wrap" label tap) diff --git a/internal/editor/auto_save_test.go b/internal/editor/auto_save_test.go index 1693cba..b8b1992 100644 --- a/internal/editor/auto_save_test.go +++ b/internal/editor/auto_save_test.go @@ -6,6 +6,7 @@ import ( "time" "pad/internal/io/pool" + "pad/internal/io/pool/mock" ) // TestWriteFailureTracking verifies that a write failure updates WriteFailed status @@ -45,7 +46,11 @@ func TestAutoSave_RetryFails(t *testing.T) { l.state.Editor.Buffer = "hello" // Mock FS to fail on Write - l.mockFS.SetWriteError(true) + mfs, ok := l.mockFS.(*mock.FileSystem) + if !ok { + t.Fatal("mockFS is not a *mock.FileSystem") + } + mfs.SetWriteError(true) // 1. Manually dispatch a write task task := pool.NewWriteFileTask(filename, []byte("hello"), l.mockFS) diff --git a/internal/editor/filename_test.go b/internal/editor/filename_test.go new file mode 100644 index 0000000..16b951c --- /dev/null +++ b/internal/editor/filename_test.go @@ -0,0 +1,54 @@ +package editor + +import ( + "testing" + "pad/internal/ui" +) + +func TestEditorLayout_Filename(t *testing.T) { + // Setup: Reset state + TheState = NewState() + + // Set a filename + expectedFilename := "test.txt" + TheState.Editor.Filename = expectedFilename + + // Run layout + screenWidth := ui.Dp(400) + screenHeight := ui.Dp(800) + elements := EditorLayout(screenWidth, screenHeight, false) + + // Find top bar (assuming it's the first container element) + topBar := elements[0].(ui.Container) + + // Find filename label (assuming it's the first label in the top bar) + filenameLabel := topBar.Children[0].(ui.Label) + + if filenameLabel.Text != expectedFilename { + t.Errorf("expected filename %q, got %q", expectedFilename, filenameLabel.Text) + } +} + +func TestEditorLayout_DefaultFilename(t *testing.T) { + // Setup: Reset state + TheState = NewState() + + // Filename is empty + TheState.Editor.Filename = "" + + // Run layout + screenWidth := ui.Dp(400) + screenHeight := ui.Dp(800) + elements := EditorLayout(screenWidth, screenHeight, false) + + // Find top bar (assuming it's the first container element) + topBar := elements[0].(ui.Container) + + // Find filename label (assuming it's the first label in the top bar) + filenameLabel := topBar.Children[0].(ui.Label) + + expectedDefault := "untitled.txt" + if filenameLabel.Text != expectedDefault { + t.Errorf("expected default filename %q, got %q", expectedDefault, filenameLabel.Text) + } +} diff --git a/internal/editor/logic.go b/internal/editor/logic.go index e5cfe04..ecf46dd 100644 --- a/internal/editor/logic.go +++ b/internal/editor/logic.go @@ -316,7 +316,7 @@ func (l *Logic) FlushAll() { // Trigger a synchronous write for the active dirty file content := l.state.Editor.Buffer // In a real app, this would be a blocking call to the FS - l.mockFS.WriteFile(filename, []byte(content)) + l.mockFS.WriteFileAtomic(filename, []byte(content)) l.state.Editor.lastWriteVersion[filename] = l.state.Editor.fileVersion[filename] } } diff --git a/internal/editor/state.go b/internal/editor/state.go index 5781a5d..11a519f 100644 --- a/internal/editor/state.go +++ b/internal/editor/state.go @@ -60,6 +60,11 @@ type EditorState struct { retryAttempts map[string]int // Added: tracks retry attempts } +// IsSaving returns true if a save is pending. +func (e *EditorState) IsSaving() bool { + return e.saveTimer != nil +} + // IsDirty, WriteFailed, and RetryAttempts are computed: func (e *EditorState) IsDirty() bool { @@ -275,11 +280,105 @@ func HandleKeyDown(data any) { HandleDelete() case key.NameReturn: HandleInsert("\n") + case key.NameHome: + HandleHome() + case key.NameEnd: + HandleEnd() + case key.NamePageUp: + HandlePageUpDown(true) + case key.NamePageDown: + HandlePageUpDown(false) } } } -// HandleVerticalCursorMove updates the cursor position to the previous or next visual line. +// HandleHome moves the cursor to the start of the current visual line. +func HandleHome() { + layout := TheState.Editor.GlyphLayout + if len(layout.ByteOffsets) == 0 { + return + } + + pos := TheState.Editor.CursorPosition + // Find current glyph index. + idx := sort.Search(len(layout.ByteOffsets), func(i int) bool { + return layout.ByteOffsets[i] >= pos + }) + if idx == len(layout.ByteOffsets) { + idx = len(layout.ByteOffsets) - 1 + } + + currentY := layout.Y[idx] + // Find first glyph on currentY. + targetIdx := idx + for i := idx; i >= 0; i-- { + if layout.Y[i] == currentY { + targetIdx = i + } else { + break + } + } + TheState.Editor.CursorPosition = layout.ByteOffsets[targetIdx] +} + +// HandleEnd moves the cursor to the end of the current visual line. +func HandleEnd() { + layout := TheState.Editor.GlyphLayout + if len(layout.ByteOffsets) == 0 { + return + } + + pos := TheState.Editor.CursorPosition + // Find current glyph index. + idx := sort.Search(len(layout.ByteOffsets), func(i int) bool { + return layout.ByteOffsets[i] >= pos + }) + if idx == len(layout.ByteOffsets) { + idx = len(layout.ByteOffsets) - 1 + } + + currentY := layout.Y[idx] + // Find last glyph on currentY. + targetIdx := idx + for i := idx; i < len(layout.Y); i++ { + if layout.Y[i] == currentY { + targetIdx = i + } else { + break + } + } + + // Position after the last character of the line. + // If it's a newline, it's the newline itself. + start := layout.ByteOffsets[targetIdx] + r, size := utf8.DecodeRuneInString(TheState.Editor.Buffer[start:]) + if r == '\n' { + TheState.Editor.CursorPosition = start + } else { + TheState.Editor.CursorPosition = start + size + } +} + +// HandlePageUpDown scrolls and moves the cursor. +func HandlePageUpDown(up bool) { + // For now, simple scrolling. Cursor movement could be added later. + pageSize := TheState.MaxScroll / 4 // Or some fraction + if pageSize < EditorLineHeight() { + pageSize = EditorLineHeight() + } + + if up { + TheState.ScrollOffset -= pageSize + if TheState.ScrollOffset < 0 { + TheState.ScrollOffset = 0 + } + } else { + TheState.ScrollOffset += pageSize + if TheState.ScrollOffset > TheState.MaxScroll { + TheState.ScrollOffset = TheState.MaxScroll + } + } +} func HandleVerticalCursorMove(up bool) { layout := TheState.Editor.GlyphLayout if len(layout.ByteOffsets) == 0 { @@ -416,7 +515,10 @@ func EditorLayout(screenWidth, screenHeight ui.Dp, wordWrap bool) []ui.Element { H: ui.Dp(52), } statusBarW := statusBarRegion.W - filename := "untitled.txt" // Needs to be managed in EditorState + filename := TheState.Editor.Filename + if filename == "" { + filename = "untitled.txt" + } statusBar := ui.NewContainer( statusBarRegion, ui.Color{R: 230, G: 230, B: 230, A: 255}, @@ -445,11 +547,20 @@ func EditorLayout(screenWidth, screenHeight ui.Dp, wordWrap bool) []ui.Element { if wordWrap { wrapText = "Wrap: On" } + statusText := "Saved" + if TheState.Editor.IsSaving() { + statusText = "Saving..." + } else if TheState.Editor.WriteFailed() { + statusText = "Error" + } else if TheState.Editor.IsDirty() { + statusText = "Modified" + } + bottomBar := ui.NewContainer( bottomBarRegion, ui.Color{R: 230, G: 230, B: 230, A: 255}, []ui.Element{ - ui.NewLabel("Ln 47, Col 12", 12, ui.Region{X: 0, Y: ui.Dp(2), W: bottomBarW, H: ui.Dp(20)}, ui.AlignStart, "", nil), + ui.NewLabel(statusText, 12, ui.Region{X: 0, Y: ui.Dp(2), W: bottomBarW, H: ui.Dp(20)}, ui.AlignStart, "", nil), ui.NewLabel("1024 / 50000", 12, ui.Region{X: 0, Y: ui.Dp(2), W: bottomBarW, H: ui.Dp(20)}, ui.AlignCenter, "", nil), ui.NewLabel(wrapText, 12, ui.Region{X: 0, Y: ui.Dp(2), W: bottomBarW, H: ui.Dp(20)}, ui.AlignEnd, "wrap", []ui.Interaction{ {Gesture: ui.Tap, Handler: ToggleWordWrap}, diff --git a/internal/io/pool/filesystem.go b/internal/io/pool/filesystem.go index 212b70a..e66f0f8 100644 --- a/internal/io/pool/filesystem.go +++ b/internal/io/pool/filesystem.go @@ -10,6 +10,7 @@ type FileSystem interface { FileExists(path string) bool ReadFile(path string) ([]byte, error) WriteFile(path string, content []byte) error + WriteFileAtomic(path string, content []byte) error DeleteFile(path string) error CreateDir(path string) error } diff --git a/internal/io/pool/mock/filesystem.go b/internal/io/pool/mock/filesystem.go index 21ba036..3e62623 100644 --- a/internal/io/pool/mock/filesystem.go +++ b/internal/io/pool/mock/filesystem.go @@ -4,7 +4,6 @@ package mock import ( "fmt" - "log" "os" "path/filepath" "sync" @@ -182,15 +181,10 @@ func (fs *FileSystem) ReadFile(path string) ([]byte, error) { return content, nil } -// WriteFile replaces the content of a file at the given path. +// WriteFile writes or replaces the content of a file at the given path. +// This is the non-atomic path — use WriteFileAtomic for safety. func (fs *FileSystem) WriteFile(path string, content []byte) error { fs.mu.Lock() - log.Printf("FileSystem: WriteFile to %s, writeError=%v", path, fs.writeError) - - if fs.writeError { - fs.mu.Unlock() - return fmt.Errorf("simulated write error") - } if fs.delay > 0 { fs.mu.Unlock() @@ -198,19 +192,108 @@ func (fs *FileSystem) WriteFile(path string, content []byte) error { fs.mu.Lock() } - f, ok := fs.files[path] - if !ok { + if fs.writeError { fs.mu.Unlock() - return fmt.Errorf("file not found: %s", path) + return fmt.Errorf("simulated write error") } - f.Content = content - f.Size = int64(len(content)) - f.ModTime = time.Now() + // Determine if this is a create or modify for notification purposes + isNew := !fs.fileExists(path) + + name := filepath.Base(path) + now := time.Now() + fs.files[path] = &File{ + Path: path, + Name: name, + Content: content, + ModTime: now, + Size: int64(len(content)), + IsDir: false, + } var events []notifyEvent if fs.changes != nil { - events = append(events, notifyEvent{path, "Modified"}) + events = append(events, notifyEvent{path, ifElse(isNew, "Created", "Modified")}) + } + + fs.mu.Unlock() + fs.sendNotifications(events) + return nil +} + +// fileExists checks if a file (not directory) exists at the given path. +// Caller must hold fs.mu. +func (fs *FileSystem) fileExists(path string) bool { + f, ok := fs.files[path] + return ok && !f.IsDir +} + +func ifElse(cond bool, a, b string) string { + if cond { + return a + } + return b +} + +// WriteFileAtomic replaces the content of a file atomically via a temp file + rename. +// Temp files live in a top-level .tmp/ directory. +func (fs *FileSystem) WriteFileAtomic(path string, content []byte) error { + fs.mu.Lock() + + if fs.delay > 0 { + fs.mu.Unlock() + time.Sleep(fs.delay) + fs.mu.Lock() + } + + if fs.writeError { + fs.mu.Unlock() + return fmt.Errorf("simulated write error") + } + + // Determine if this is a create or modify for notification purposes + isNew := !fs.fileExists(path) + + // Temp file path: .tmp/ + tempPath := ".tmp/" + filepath.Base(path) + + // Ensure .tmp/ directory exists + if _, ok := fs.files[".tmp"]; !ok { + fs.files[".tmp"] = &File{ + Path: ".tmp", + Name: ".tmp", + ModTime: time.Now(), + IsDir: true, + } + } + + name := filepath.Base(path) + now := time.Now() + + // 1. Write to temp location + fs.files[tempPath] = &File{ + Path: tempPath, + Name: name + ".tmp", + Content: content, + ModTime: now, + Size: int64(len(content)), + IsDir: false, + } + + // 2. Atomic rename: insert target + delete temp in one locked operation + fs.files[path] = &File{ + Path: path, + Name: name, + Content: content, + ModTime: now, + Size: int64(len(content)), + IsDir: false, + } + delete(fs.files, tempPath) + + var events []notifyEvent + if fs.changes != nil { + events = append(events, notifyEvent{path, ifElse(isNew, "Created", "Modified")}) } fs.mu.Unlock() diff --git a/internal/io/pool/mock/filesystem_test.go b/internal/io/pool/mock/filesystem_test.go index 06737f5..5f00371 100644 --- a/internal/io/pool/mock/filesystem_test.go +++ b/internal/io/pool/mock/filesystem_test.go @@ -71,12 +71,20 @@ func TestFileSystem_WriteFile(t *testing.T) { } } -func TestFileSystem_WriteFile_NotFound(t *testing.T) { +func TestFileSystem_WriteFile_CreatesFile(t *testing.T) { fs := NewFileSystem() - err := fs.WriteFile("/nonexistent.txt", []byte("data")) - if err == nil { - t.Fatal("Expected error for nonexistent file, got nil") + err := fs.WriteFile("/newfile.txt", []byte("data")) + if err != nil { + t.Fatalf("WriteFile should create file, got error: %v", err) + } + + content, err := fs.ReadFile("/newfile.txt") + if err != nil { + t.Fatalf("File should exist after write: %v", err) + } + if string(content) != "data" { + t.Errorf("Content = %q, want %q", string(content), "data") } } diff --git a/internal/io/pool/real/filesystem.go b/internal/io/pool/real/filesystem.go index 915ed1a..40b190f 100644 --- a/internal/io/pool/real/filesystem.go +++ b/internal/io/pool/real/filesystem.go @@ -38,9 +38,16 @@ func (fs *RealFileSystem) ReadFile(path string) ([]byte, error) { } func (fs *RealFileSystem) WriteFile(path string, content []byte) error { + // Simple write for backward compatibility if needed, + // but defer to Atomic implementation. + return fs.WriteFileAtomic(path, content) +} + +func (fs *RealFileSystem) WriteFileAtomic(path string, content []byte) error { // Atomic write implementation fullPath := filepath.Join(fs.Root, path) - tmpPath := filepath.Join(fs.Root, ".tmp", filepath.Base(path)) + // Temp file in the same directory as the target to ensure same filesystem rename + tmpPath := filepath.Join(filepath.Dir(fullPath), "."+filepath.Base(path)+".tmp") if err := os.MkdirAll(filepath.Dir(tmpPath), 0755); err != nil { return err diff --git a/internal/io/pool/task.go b/internal/io/pool/task.go index f0728b8..bd6b48a 100644 --- a/internal/io/pool/task.go +++ b/internal/io/pool/task.go @@ -337,7 +337,7 @@ func NewWriteFileTask(path string, data []byte, fs FileSystem) *WriteFileTask { } func (t *WriteFileTask) Execute() Result { - err := t.FS.WriteFile(t.Path, t.Data) + err := t.FS.WriteFileAtomic(t.Path, t.Data) if err != nil { return Result{ TaskID: t.taskID, diff --git a/internal/io/pool/task_test.go b/internal/io/pool/task_test.go index f82862a..b989321 100644 --- a/internal/io/pool/task_test.go +++ b/internal/io/pool/task_test.go @@ -146,14 +146,23 @@ func TestWriteFileTask_Execute(t *testing.T) { } } -func TestWriteFileTask_Execute_NotFound(t *testing.T) { +func TestWriteFileTask_Execute_CreatesFile(t *testing.T) { fs := mock.NewFileSystem() - task := NewWriteFileTask("/nonexistent.txt", []byte("data"), fs) + task := NewWriteFileTask("/newfile.txt", []byte("data"), fs) result := task.Execute() - if result.IsSuccess() { - t.Fatal("Expected failure for nonexistent file") + if !result.IsSuccess() { + t.Fatalf("Expected success (write creates file), got error: %v", result.Error) + } + + // Verify the file was actually created + content, err := fs.ReadFile("/newfile.txt") + if err != nil { + t.Fatalf("File should exist after write: %v", err) + } + if string(content) != "data" { + t.Errorf("Content = %q, want %q", string(content), "data") } } diff --git a/internal/ui/element.go b/internal/ui/element.go index 8019b76..c9111f6 100644 --- a/internal/ui/element.go +++ b/internal/ui/element.go @@ -45,7 +45,7 @@ type Container struct { region Region visible bool background Color - children []Element + Children []Element // Changed to Exported } func (c Container) Type() string { return "container" } @@ -61,8 +61,8 @@ func (c Container) Draw(gtx layout.Context, r *Renderer) { // String returns a string representation of the Container and all children. func (c Container) String() string { var sb strings.Builder - sb.WriteString(fmt.Sprintf("Container[%s] region=%+v bg=%#v children=%d", c.id, c.region, c.background, len(c.children))) - for i, child := range c.children { + sb.WriteString(fmt.Sprintf("Container[%s] region=%+v bg=%#v children=%d", c.id, c.region, c.background, len(c.Children))) + for i, child := range c.Children { if str, ok := any(child).(fmt.Stringer); ok { sb.WriteString(fmt.Sprintf("\n [%d] %s", i, str.String())) } else { @@ -78,7 +78,7 @@ func NewContainer(region Region, bg Color, children []Element) Container { region: region, visible: true, background: bg, - children: children, + Children: children, } } diff --git a/internal/ui/render.go b/internal/ui/render.go index 4bc7375..c68846f 100644 --- a/internal/ui/render.go +++ b/internal/ui/render.go @@ -257,7 +257,7 @@ func (r *Renderer) drawElement(gtx layout.Context, e Element) { }.Push(gtx.Ops) e.Draw(gtx, r) // draw container background offset := op.Offset(image.Pt(int(r.toPx(reg.X)), int(r.toPx(reg.Y)))).Push(gtx.Ops) - for _, child := range container.children { + for _, child := range container.Children { r.drawElement(gtx, child) } offset.Pop()