Fix test build after NewLogic signature change; add development plan draft
NewLogic now takes (FileSystem, path, openfunc); update the e2e harness and editor tests accordingly. The harness uses a no-op openfunc, matching impl_other.go (the real app's OpenFile is a platform bridge, no-op off Android). Also adds doc/development_plan.md (draft, with corrections note re: the live repo state).
This commit is contained in:
parent
4052769813
commit
03fb63863c
280
doc/development_plan.md
Normal file
280
doc/development_plan.md
Normal file
|
|
@ -0,0 +1,280 @@
|
|||
# Development Plan: from skeleton to lean, usable app
|
||||
|
||||
Status: DRAFT — written 2026-08-15 against the stale Backup snapshot (May 31).
|
||||
The live repo (~/pad) is 47 commits ahead; see the "Corrections" note at the
|
||||
bottom before acting on anything in this document.
|
||||
|
||||
## 1. Decision summary
|
||||
|
||||
- **Tooling:** delete the hand-rolled `internal/ui` entirely. Build on Gio's native
|
||||
widgets: `widget.Editor`, `widget.List`, `layout.List`, `widget.Button`,
|
||||
`widget.RichText`. This removes the entire bug class the project has been
|
||||
fighting (positioning/scroll/clipping/gesture) and is the *only* path to Android
|
||||
IME input (swipe typing, autocorrect) without writing Android InputConnection
|
||||
code ourselves.
|
||||
- **Concurrency:** one main goroutine owns all app state and does all drawing.
|
||||
File I/O runs in background goroutines; results come back over a channel and are
|
||||
applied on the main goroutine. No logic goroutine, no unbuffered
|
||||
display-channels, no globals (`editor.TheState`, `browser.currentBrowserState`,
|
||||
`ui.OpenFile` all die).
|
||||
- **Target size limit:** 50 MB files — but stock `widget.Editor` cannot do this
|
||||
(evidence below). The plan stages this as an explicit decision point with three
|
||||
options; recommendation: ship with stock editor at a documented limit, and do
|
||||
windowed shaping as a separate, isolated project if 50 MB is still required.
|
||||
|
||||
## 2. Evidence (verified against gioui.org v0.9.0 sources)
|
||||
|
||||
1. **IME works through `widget.Editor` only.** `GioView.java` implements
|
||||
`onCreateInputConnection` → `GioInputConnection` (commitText /
|
||||
setComposingText / setComposingRegion); `app/ime.go` exposes
|
||||
`editorState.Replace`; `widget.Editor` pushes `key.SnippetCmd` every frame.
|
||||
Our current custom UI never receives IME input at all.
|
||||
2. **The IME snippet is selection-scoped** (`updateSnippet` reads only the
|
||||
selected range; empty selection → empty snippet at caret). Consequence:
|
||||
windowing the editor's backing text does *not* break IME behavior.
|
||||
3. **`widget.Editor` is not virtualized.** `textView.layoutText` shapes the
|
||||
*entire* document on every invalidation (each keystroke, IME commit, wrap
|
||||
toggle, width change — *not* on scroll; scroll only moves a clip) and builds
|
||||
an in-memory `glyphIndex` of **~200 bytes per rune** (`combinedPos` +
|
||||
`text.Glyph` per rune).
|
||||
4. **Measured on this desktop (shaper.Layout + full glyph iteration, = the
|
||||
per-keystroke cost of `widget.Editor`):**
|
||||
|
||||
| text size | time/keystroke | peak RAM |
|
||||
|---|---|---|
|
||||
| 1 MB | 17 ms | ~0.32 GB |
|
||||
| 2 MB | ~35 ms (extrapolated) | ~0.9 GB |
|
||||
| 3 MB | — | ~1.7 GB (measured, incl. GC churn) |
|
||||
| 4 MB | — | OOM-killed the benchmark host |
|
||||
|
||||
Planning figure: **~0.5 GB of RAM per 1 MB of text** (editor index +
|
||||
shaper store + buffer + undo copies, with reallocation churn). Android app
|
||||
memory ceilings (lmkd) are typically ~1.5–2 GB on 8 GB phones.
|
||||
|
||||
Therefore: comfortable editing up to **~2 MB**, a hard guard at **5 MB**
|
||||
(≈ 2.5 GB — survivable on most phones, heavy), 10 MB ≈ 5 GB (OOM), 50 MB is
|
||||
not deliverable with stock `widget.Editor`. 10 MB editing requires windowed
|
||||
shaping (fork-level work on Gio). See §6.
|
||||
|
||||
## 3. What gets deleted
|
||||
|
||||
| Item | Where | Why |
|
||||
|---|---|---|
|
||||
| Whole custom toolkit | `internal/ui/` (element.go, render.go, layout.go, unit.go, icons/) ~1,500 L | replaced by Gio widgets |
|
||||
| Worker pool + mock FS service | `internal/io/pool/worker_pool.go`, `mock/` (429 L) | replaced by plain background goroutines |
|
||||
| 10 of 11 task types | `internal/io/pool/task.go` | only Read/Write are ever used, as plain functions |
|
||||
| On-disk JSON index cache | `internal/browser/index.go` `.pad/indices/` machinery | cache-in-indexed-dir is the fragile design behind the failing mtime test; a sync directory needs no cache (re-scan is ms) |
|
||||
| Dead browser state | `state.go`: `VisibleCount` (never set → list rendered 0 rows), `RowFilenames`, `Pages`, `Page`, `SortIndex`, `SortMode` (duplicate of browser's), `PaginationState`, `PageSize` | replaced by `widget.List` + real filenames |
|
||||
| Untyped handlers | `browser/handlers.go`, `editor/logic.go` `func(any)` closures + globals | replaced by direct calls on the main goroutine |
|
||||
| e2e harness | `internal/test/e2e/` | written against the dead architecture; replaced by widget-level tests after Phase 1 |
|
||||
| Stale docs | `element_model.md`, `layout_rendering.md`, `touch.md`, `conflict_resolution.md`, `browser_implementation_plan.md` | describe deleted systems; `architecture.md` rewritten last |
|
||||
|
||||
**Kept (ported, not rewritten):** `browser/types.go` (Entry), `sort.go` + tests,
|
||||
`search.go` + tests, directory scan from `index.go` (minus cache), the wrap /
|
||||
autosave / per-file-offset concepts from `editor/state.go`.
|
||||
|
||||
## 4. Target architecture (single diagram, no channels except I/O)
|
||||
|
||||
```
|
||||
main goroutine (Gio frame loop, owns ALL state)
|
||||
app.State {
|
||||
page: browser | editor
|
||||
dir: string // current browser directory
|
||||
entries: []browser.Entry // sorted+filtered, rebuilt on demand
|
||||
sort: browser.SortMode
|
||||
query: string
|
||||
file: openFile{path, modTime, wrap, editor widget.Editor, dirty, lastEdit}
|
||||
restore: state.RestoreData // persisted JSON
|
||||
}
|
||||
on FrameEvent:
|
||||
- handle I/O results from chan result
|
||||
- draw current page
|
||||
background goroutines (one per op):
|
||||
os.ReadFile / os.WriteFile / dir scan → send result on chan
|
||||
```
|
||||
|
||||
- **Browser page:** top bar (path, sort menu) + search field +
|
||||
`layout.List` over `entries` (virtualized; 1,000-file directory is a
|
||||
non-issue). Tap → start background read → on result, switch to editor page.
|
||||
- **Editor page:** `widget.Editor` bound to the file's text; toolbar (filename,
|
||||
wrap toggle, close). Autosave: `dirty` + 1 s debounce in the frame loop,
|
||||
background write, status shows saved/dirty. Undo/redo in-session: built into
|
||||
`widget.Editor` (unexported history, triggered by normal editing) — no work.
|
||||
- **State restore** (spec: survive close/reopen): small JSON in the app data
|
||||
dir: `{openFile, wrap, autosave, offsets: {path: {line, col}}}`. On open:
|
||||
`SetText` + `SetCaret(line, col)`. `SetCaret` auto-scrolls to the caret.
|
||||
Offsets are rune offsets — invalid if the file changed; validate against
|
||||
persisted mtime and fall back to top on mismatch.
|
||||
- **External change detection:** on open and on app resume, compare mtime to
|
||||
the opened modTime; if changed and we're not dirty → reload; if dirty →
|
||||
one-line banner with Keep / Reload (no full conflict UI in v1).
|
||||
- **File-size guard:** on open, if `size > limit` → show "file too large to
|
||||
edit in v1" (limit per §6 decision).
|
||||
- **Testability:** state transitions (open file, sort, search, autosave due,
|
||||
external-change decision) are plain functions on `app.State` → directly
|
||||
unit-testable. Device behavior covered by manual checklist + `-race` in CI.
|
||||
|
||||
## 5. Phases
|
||||
|
||||
### Phase 0 — deletion-only (no new features)
|
||||
1. Delete everything in §3.
|
||||
2. `cmd/pad` keeps an `app.Window` with a placeholder screen; build green.
|
||||
3. Keep passing tests by porting sort/search tests to the trimmed `browser`
|
||||
package; delete the on-disk-cache test (feature deleted) — this removes the
|
||||
flaky `TestBuildIndex_CacheInvalidatedOnMtimeChange` by deleting the cache.
|
||||
|
||||
**Done when:** `go build ./... && go vet ./... && go test -race ./...` green;
|
||||
repo ≤ ~2,500 lines of Go; no `func(any)`, no package-level mutable state,
|
||||
no channels except (added in Phase 1) the I/O result channel.
|
||||
|
||||
### Phase 1 — browser
|
||||
1. `layout.List` browser page: rows = name + modified time; directories
|
||||
distinguished; tap on dir descends, breadcrumb back; tap on file opens it
|
||||
(Phase 2 stub: toast "no editor yet").
|
||||
2. Port sort (name/modified, asc/desc) + search filter onto the page.
|
||||
3. Background dir-scan for large directories; version counter to drop stale
|
||||
results.
|
||||
|
||||
**Done when:** on-device: browse a real 1,000+ file directory at full frame
|
||||
rate; sort and search correct (unit tests); no data races.
|
||||
|
||||
### Phase 2 — editor
|
||||
1. Open file → background read → `widget.Editor` with `SetText`; toolbar with
|
||||
wrap toggle (maps to `WrapPolicy`) and close (back to browser).
|
||||
2. Autosave: debounce, background write, dirty indicator.
|
||||
3. State restore + per-file offset memory + external-change check (§4).
|
||||
4. IME verification on device: swipe typing, autocorrect, multi-line commits,
|
||||
caret jump after commit.
|
||||
|
||||
**Done when:** edit + close + reopen restores file and caret; kill app,
|
||||
reopen, same; external modification handled per §4; `-race` clean.
|
||||
|
||||
### Phase 3 — the 50 MB decision (see §6)
|
||||
Either (A) document the limit and ship, or (C) build and integrate the
|
||||
windowed editor. (B, full chunked streaming, is out of scope — it implies a
|
||||
custom editor and forfeits the IME advantage.)
|
||||
|
||||
### Phase 4 — hardening
|
||||
1. Rewrite `doc/architecture.md` to match reality; amend `spec.md` per §7.
|
||||
2. `README.md`: what it is, how to build/install (Go 1.24, gioui.org, NDK).
|
||||
3. Device test checklist in-repo.
|
||||
|
||||
## 6. The 50 MB decision (explicit fork)
|
||||
|
||||
| Option | What | IME | Effort | Outcome |
|
||||
|---|---|---|---|---|
|
||||
| **A. Accept a documented limit** (2 MB soft warn, 5 MB hard guard — §2.4) | nothing more than Phase 2 | full | done | the large majority of real text files in a sync dir; honest limit in UI |
|
||||
| **C. Windowed editor** | isolated project: shape only viewport ± window; `glyphIndex` covers the window; offset map rune→window; scroll triggers re-window; undo history kept as per-operation edits, not full copies | full (snippet is selection-scoped, verified) | large — touches Gio internals (`textView`, `Editor`), upstreamable but expect divergence | 10 MB+ editing at 60 fps, *if* the PoC holds |
|
||||
| B. Chunked streaming editor (spec's original) | build our own editor on raw ops | **lost** (no InputConnection) | largest | rejected: forfeits the core reason for moving to native |
|
||||
|
||||
**Recommendation: A now, C later as its own branch.** Reasoning: A delivers a
|
||||
usable app; the Phase 0–2 work is identical under either choice, so nothing is
|
||||
locked in. C is only worth starting if, after real usage, the 5 MB guard bites
|
||||
(i.e. we genuinely need to *edit* larger files). If C is started, gate it
|
||||
behind a PoC first: windowed shaping of a 10 MB file must sustain 60 fps
|
||||
scroll + sub-100 ms keystroke on the target phone before any integration work.
|
||||
|
||||
## 7. Spec deltas (to write into spec.md after Phase 2)
|
||||
|
||||
1. "No practical limits / file never fully in memory" → "files up to N MB
|
||||
(measured); larger files openable in browser, not editable."
|
||||
2. Undo across sessions → dropped (in-session undo/redo is provided by the
|
||||
editor widget).
|
||||
3. Add: IME/swipe/autocorrect support (was an implicit requirement, now
|
||||
explicit and delivered).
|
||||
4. Add: external-change detection behavior (§4) as the conflict policy,
|
||||
replacing `conflict_resolution.md`.
|
||||
|
||||
## 8. Non-goals (v1)
|
||||
|
||||
- File-system watcher / live browser refresh (re-scan on return to browser).
|
||||
- Multi-file views, tabs, split.
|
||||
- Syntax highlighting, search-in-file, diff/merge UI.
|
||||
- Desktop/other platforms (Android-first; window sized per existing
|
||||
`unit.Dp(390) × unit.Dp(844)`).
|
||||
|
||||
## 9. Testing strategy (replaces `internal/test/e2e`)
|
||||
|
||||
The old e2e harness is deleted: it existed to test the channel/frame
|
||||
choreography, which no longer exists. E2E regression still works, in three
|
||||
layers:
|
||||
|
||||
**Layer 1 — Go unit tests on pure state functions (the bulk).** The design
|
||||
keeps every behavior as a plain function on `app.State` (no goroutines, no
|
||||
widgets), so the regression surface is directly testable on a host in
|
||||
milliseconds:
|
||||
- sort (all modes) / search / directory scan — port the existing
|
||||
`browser` package tests;
|
||||
- `applyReadResult` / `applyWriteResult` / `applyScanResult`, including
|
||||
**stale/out-of-order results** (version counters) — the exact race the old
|
||||
channel design was prone to, now a synchronous decision under test;
|
||||
- autosave debounce and dirty-flag decisions (clock injected, no real timers);
|
||||
- restore JSON encode/decode; offset validation against mtime; external-change
|
||||
decision table (changed+clean → reload; changed+dirty → prompt;
|
||||
unchanged → open);
|
||||
- file-size guard (2 MB warn / 5 MB reject).
|
||||
|
||||
CI gate: `go build && go vet && go test -race ./...` on every commit.
|
||||
|
||||
**Layer 2 — UI code thin by construction.** Gio has no headless/test window
|
||||
(verified in v0.9 and v0.10), so widget-level Go tests are not possible.
|
||||
Mitigation is structural, not test-based: draw functions are ~30-line wirings
|
||||
of `layout.List` / `widget.Editor` over `app.State`, and all positioning math
|
||||
lives inside Gio. The bug class the old UI had (positioning/scroll/clipping)
|
||||
is *deleted*, not covered by tests.
|
||||
|
||||
**Layer 3 — scripted on-device e2e: an in-app selftest.** A debug-build flag
|
||||
(`pad --selftest`, or launched via `am start` with an extra) drives the **real
|
||||
state machine through the real draw path**:
|
||||
1. create a fixture tree under the app's data dir;
|
||||
2. browser: assert entry count/names in the rendered state;
|
||||
3. open a file: assert editor text == fixture content;
|
||||
4. force the autosave deadline: assert the file on disk is rewritten;
|
||||
5. save restore state, simulate reopen: assert file + caret offset restored;
|
||||
6. touch the file's mtime externally, reopen: assert the reload-prompt
|
||||
decision fires.
|
||||
Each step logs PASS/FAIL; a summary is written to a file the host pulls via
|
||||
adb and asserts on. Deterministic, seconds long, runnable in CI against a
|
||||
farm/one attached device.
|
||||
|
||||
**Manual-only (unavoidable):** swipe typing, autocorrect, caret jump after an
|
||||
IME commit — `adb input text` bypasses the IME entirely, so these cannot be
|
||||
scripted by any tool. They live in a short checklist in the repo, run before
|
||||
releases, alongside the §9/DoD journey.
|
||||
|
||||
## 10. Definition of done for the whole effort
|
||||
|
||||
On a real Android phone: launch → previous file and caret restored → browse a
|
||||
large directory smoothly → open a real file → swipe-type with autocorrect →
|
||||
close and reopen, caret where you left it → modify file externally, get the
|
||||
reload prompt → autosave lands on disk within ~1 s. Repo: ≤ ~3,000 lines of
|
||||
Go, one channel, zero globals, `go test -race ./...` green.
|
||||
|
||||
---
|
||||
|
||||
## Corrections (live repo, ~/pad, as of 2026-08-15)
|
||||
|
||||
The live repo has advanced far beyond the snapshot this document analyzed:
|
||||
real filesystem (`internal/io/pool/real/`), a built Android APK (JNI +
|
||||
permissions), a working editor (click-to-cursor, Enter/Backspace, autosave),
|
||||
and a **chunked buffer** (`internal/editor/chunked_buffer.go`) implementing the
|
||||
spec's "file never fully in memory" approach that this plan assumed would
|
||||
require fork-level work. Concretely:
|
||||
|
||||
- `VisibleCount` is now assigned (`internal/editor/state.go:174`); the
|
||||
"browser renders 0 rows" finding is stale.
|
||||
- The IME path is half-wired at the raw-op layer: `cmd/pad/main.go` consumes
|
||||
`key.EditEvent`/`key.SnippetEvent` and fires `key.SoftKeyboardCmd`, but
|
||||
**nothing creates the window-level editor-state op** (`app.EditorState`), so
|
||||
IME *text commits* (swipe, autocorrect) likely still have nowhere to land;
|
||||
raw key events (Enter/Backspace) work. Completing this wiring is the
|
||||
pivotal experiment (see emulator decision below).
|
||||
- The live test suite is RED: `internal/editor` and `internal/test/e2e` fail
|
||||
to build after `NewLogic`'s signature changed to
|
||||
`(pool.FileSystem, string, func(string))`.
|
||||
- The A/B fork is now: **(A)** this plan's widget rebuild, vs **(B)** finish
|
||||
the live path (complete IME wiring, stabilize chunked buffer, fix tests).
|
||||
The emulator swipe-typing experiment decides.
|
||||
|
||||
Decision pending: Path A vs Path B. Until decided, this document is a
|
||||
reference for the widget-rebuild option, not an active plan.
|
||||
|
|
@ -33,7 +33,7 @@ func TestWriteFailureTracking(t *testing.T) {
|
|||
// TestAutoSave_RetryFails verifies that failed writes trigger retry mechanism.
|
||||
func TestAutoSave_RetryFails(t *testing.T) {
|
||||
// Setup: New Logic, set mock FS to fail
|
||||
l := NewLogic(nil)
|
||||
l := NewLogic(nil, "/", func(string) {})
|
||||
|
||||
// Start a goroutine to drain frameChan to prevent deadlocks
|
||||
go func() {
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ func TestAutoSaveE2E(t *testing.T) {
|
|||
initialContent := "Hello"
|
||||
mockFS.AddFile(filename, []byte(initialContent), time.Now())
|
||||
|
||||
l := NewLogic(mockFS)
|
||||
l := NewLogic(mockFS, "/", func(string) {})
|
||||
go l.Run()
|
||||
defer l.Done()
|
||||
|
||||
|
|
@ -93,7 +93,7 @@ func TestLargeFileChunkBoundary(t *testing.T) {
|
|||
}
|
||||
mockFS.AddFile(filename, initialContent, time.Now())
|
||||
|
||||
l := NewLogic(mockFS)
|
||||
l := NewLogic(mockFS, "/", func(string) {})
|
||||
go l.Run()
|
||||
defer l.Done()
|
||||
|
||||
|
|
@ -232,7 +232,7 @@ func TestFlushOnExitE2E(t *testing.T) {
|
|||
initialContent := "Hello"
|
||||
mockFS.AddFile(filename, []byte(initialContent), time.Now())
|
||||
|
||||
l := NewLogic(mockFS)
|
||||
l := NewLogic(mockFS, "/", func(string) {})
|
||||
go l.Run()
|
||||
defer l.Done()
|
||||
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ func TestOpenFileIntegration(t *testing.T) {
|
|||
content := "Hello World"
|
||||
mockFS.AddFile(filename, []byte(content), time.Now())
|
||||
|
||||
l := NewLogic(mockFS)
|
||||
l := NewLogic(mockFS, "/", func(string) {})
|
||||
go l.Run()
|
||||
defer l.Done()
|
||||
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ type HarnessOption func(*Harness)
|
|||
// NewHarness creates a test harness with the given options.
|
||||
func NewHarness(opts ...HarnessOption) *Harness {
|
||||
h := &Harness{
|
||||
logic: editor.NewLogic(nil),
|
||||
logic: editor.NewLogic(nil, "/", func(string) {}), // no-op openfunc (matches impl_other.go)
|
||||
capture: NewFrameCapture(),
|
||||
frameReceiverDone: make(chan struct{}),
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user