Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/ui/diffview_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,7 @@ func TestModel_PlainStyles(t *testing.T) {
m.layout.height = 40
m.layout.treeWidth = 36
m.ready = true
m.filesLoaded = true
// plain styles should not panic and should render
output := m.View()
assert.NotEmpty(t, output)
Expand Down
15 changes: 13 additions & 2 deletions app/ui/loaders.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,16 @@ import (

// loadFiles returns a command that fetches the list of changed files from the renderer.
// it also appends staged-only and untracked files when applicable.
// the caller must bump m.filesLoadSeq before invoking loadFiles when issuing a new
// reload (e.g. toggleUntracked); the captured seq tags every emitted filesLoadedMsg
// so handleFilesLoaded can drop stale results from earlier in-flight loads.
func (m Model) loadFiles() tea.Cmd {
seq := m.filesLoadSeq
return func() tea.Msg {
var warnings []string
entries, err := m.diffRenderer.ChangedFiles(m.cfg.ref, m.cfg.staged)
if err != nil {
return filesLoadedMsg{entries: entries, err: err}
return filesLoadedMsg{seq: seq, entries: entries, err: err}
}
// include staged-only files (new files added to index but not yet committed)
// only when there are no unstaged entries; otherwise unstaged review should stay focused
Expand Down Expand Up @@ -58,7 +62,7 @@ func (m Model) loadFiles() tea.Cmd {
}
}
}
return filesLoadedMsg{entries: entries, warnings: warnings}
return filesLoadedMsg{seq: seq, entries: entries, warnings: warnings}
}
}

Expand Down Expand Up @@ -99,6 +103,13 @@ func (m Model) loadSelectedIfChanged() (tea.Model, tea.Cmd) {
// handleFilesLoaded processes the result of loadFiles, populating the file tree
// and triggering the initial file diff load.
func (m Model) handleFilesLoaded(msg filesLoadedMsg) (tea.Model, tea.Cmd) {
// drop stale responses from an earlier load; only the latest load request
// (matching m.filesLoadSeq) is accepted. Prevents an older in-flight load from
// overwriting the tree after toggleUntracked (or a rapid double-toggle).
if msg.seq != m.filesLoadSeq {
return m, nil
}
m.filesLoaded = true
if msg.err != nil {
m.layout.viewport.SetContent(fmt.Sprintf("error loading files: %v", msg.err))
return m, nil
Expand Down
38 changes: 38 additions & 0 deletions app/ui/loaders_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,50 @@ func TestModel_FilesLoaded(t *testing.T) {
func TestModel_FilesLoadedError(t *testing.T) {
m := testModel(nil, nil)
m.ready = true
m.filesLoaded = false

result, cmd := m.Update(filesLoadedMsg{err: assert.AnError})
model := result.(Model)

assert.Nil(t, cmd)
assert.Equal(t, 0, model.tree.TotalFiles())
// filesLoaded must flip even on error, otherwise View() stays on "loading files..." forever
assert.True(t, model.filesLoaded, "filesLoaded must be set before the error early-return so the loading screen exits")
}

func TestModel_FilesLoaded_DropsStaleResponses(t *testing.T) {
// regression: a slow first load (seq=0) must not overwrite the tree after
// a newer load (seq=1) was issued — e.g. user toggled untracked immediately after startup.
m := testModel(nil, nil)
m.filesLoaded = false
m.filesLoadSeq = 1 // simulate a newer load already dispatched (e.g. toggleUntracked)

// stale response (seq=0) arrives first — must be dropped
stale := []diff.FileEntry{{Path: "stale.go"}}
result, cmd := m.Update(filesLoadedMsg{seq: 0, entries: stale})
model := result.(Model)
assert.Nil(t, cmd)
assert.False(t, model.filesLoaded, "stale response must not flip filesLoaded")
assert.Equal(t, 0, model.tree.TotalFiles(), "stale entries must not populate tree")
assert.Equal(t, "loading files...", model.View(), "View must still show loading while the current load is pending")

// fresh response (seq=1) arrives — accepted
fresh := []diff.FileEntry{{Path: "fresh.go"}}
result, _ = m.Update(filesLoadedMsg{seq: 1, entries: fresh})
model = result.(Model)
assert.True(t, model.filesLoaded)
assert.Equal(t, 1, model.tree.TotalFiles())
}

func TestModel_ToggleUntrackedBumpsFilesLoadSeq(t *testing.T) {
// regression: toggleUntracked must bump filesLoadSeq so any in-flight load
// from before the toggle is treated as stale by handleFilesLoaded.
m := testModel(nil, nil)
m.loadUntracked = func() ([]string, error) { return nil, nil }
before := m.filesLoadSeq
cmd := m.toggleUntracked()
require.NotNil(t, cmd)
assert.Greater(t, m.filesLoadSeq, before, "toggleUntracked must bump filesLoadSeq")
}

func TestModel_FilesLoadedMultipleFiles(t *testing.T) {
Expand Down
6 changes: 5 additions & 1 deletion app/ui/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,9 @@ type Model struct {
search searchState // search lifecycle state
annot annotationState // annotation input lifecycle state

ready bool // true after first WindowSizeMsg
ready bool // true after first WindowSizeMsg
filesLoaded bool // true after the first filesLoadedMsg is handled (keeps the loading view pinned until real data arrives)
filesLoadSeq uint64 // bumped before each new file-list load; stale filesLoadedMsg (seq mismatch) is dropped

blamer Blamer // optional blame provider (nil when git unavailable)
loadUntracked func() ([]string, error) // fetches untracked files; nil when unavailable
Expand Down Expand Up @@ -349,6 +351,7 @@ type blameLoadedMsg struct {

// filesLoadedMsg is sent when the changed file list is loaded.
type filesLoadedMsg struct {
seq uint64 // matches m.filesLoadSeq at the time the load was issued; mismatched messages are dropped
entries []diff.FileEntry
err error
warnings []string // non-fatal issues (staged/untracked fetch failures)
Expand Down Expand Up @@ -790,6 +793,7 @@ func (m *Model) toggleWordDiff() {
// toggleUntracked toggles visibility of untracked files in the tree.
func (m *Model) toggleUntracked() tea.Cmd {
m.modes.showUntracked = !m.modes.showUntracked
m.filesLoadSeq++
return m.loadFiles()
}

Expand Down
27 changes: 27 additions & 0 deletions app/ui/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ func testModel(files []string, fileDiffs map[string][]diff.DiffLine) Model {
m.layout.height = 40
m.layout.treeWidth = m.layout.width * m.cfg.treeWidthRatio / 10
m.ready = true
m.filesLoaded = true
return m
}

Expand Down Expand Up @@ -264,6 +265,32 @@ func TestModel_Init(t *testing.T) {
assert.NoError(t, flm.err)
}

func TestModel_InitialLoadingState_NoEmptyFlash(t *testing.T) {
// regression: before filesLoadedMsg is handled, View() must not paint the two-pane
// layout with an empty tree and "no file selected". That made users see a 100-500 ms
// flash of a "no changes" screen on launch. See app/ui/view.go and handleFilesLoaded.
m := testModel([]string{"a.go", "b.go"}, nil)
// undo the testModel shortcut to simulate a real program start
m.ready = false
m.filesLoaded = false

// before WindowSizeMsg: generic loading string
assert.Equal(t, "loading...", m.View())

// WindowSizeMsg arrives; filesLoadedMsg has not
result, _ := m.Update(tea.WindowSizeMsg{Width: 120, Height: 40})
m = result.(Model)
assert.Equal(t, "loading files...", m.View())

// filesLoadedMsg arrives; loading state must end
result, _ = m.Update(filesLoadedMsg{entries: []diff.FileEntry{{Path: "a.go"}, {Path: "b.go"}}})
m = result.(Model)
assert.True(t, m.filesLoaded)
got := m.View()
assert.NotEqual(t, "loading...", got)
assert.NotEqual(t, "loading files...", got)
}

func TestModel_EnterSwitchesToDiffPane(t *testing.T) {
lines := []diff.DiffLine{{NewNum: 1, Content: "line1", ChangeType: diff.ChangeContext}}
m := testModel([]string{"a.go", "b.go"}, map[string][]diff.DiffLine{"a.go": lines})
Expand Down
7 changes: 7 additions & 0 deletions app/ui/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ func (m Model) View() string {
if !m.ready {
return "loading..."
}
// ready but the first filesLoadedMsg hasn't landed yet — the file tree is still
// nil-populated and the diff pane has no file selected. Showing the empty two-pane
// layout here would flash a misleading "no changes" state for as long as ChangedFiles
// takes to return (can be 100-500ms on large repos).
if !m.filesLoaded {
return "loading files..."
}

ph := m.paneHeight()

Expand Down
13 changes: 12 additions & 1 deletion docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -248,13 +248,24 @@ main() [main.go]
```
Model.Init() → loadFiles cmd
→ Renderer.ChangedFiles() → filesLoadedMsg
→ tree.Rebuild(entries)
→ handleFilesLoaded drops stale msg (seq mismatch), else m.filesLoaded = true
→ (on success) tree.Rebuild(entries)
→ auto-select first file → loadFileDiff cmd
→ Renderer.FileDiff() → fileLoadedMsg
→ highlight.HighlightLines() → highlightedLines
→ (optional) loadBlame cmd → blameLoadedMsg
```

`View()` is gated on two flags so the user never sees an empty two-pane layout
during async initialisation: it returns the literal `"loading..."` while
`!m.ready` (before the first `WindowSizeMsg`), then `"loading files..."` while
`m.ready && !m.filesLoaded` (after resize, before an accepted `filesLoadedMsg`).
`filesLoaded` flips to true on every accepted `filesLoadedMsg` (success or
error), so the loading screen always exits once the current in-flight load
returns. Stale responses (`msg.seq != m.filesLoadSeq`, e.g. an older load still
in flight after `toggleUntracked` bumped the sequence) are dropped and do not
flip the flag or rebuild the tree.

### Rendering Pipeline

```
Expand Down