Skip to content

Commit ec652ae

Browse files
committed
fix: address crosscheck review findings
- Follow mode: anchor to current file when preceding files disappear, clamp to adjacent position only when current file is gone - Follow mode: replace goto with labeled break for idiomatic Go - Baseline reset: clear LastChangedPath to prevent stale follow jumps - Baseline reset: send empty update on ComputeDiff failure to clear UI - Add TODO(v2) comments for isIgnored and getUntrackedDiff performance
1 parent a01ade6 commit ec652ae

5 files changed

Lines changed: 94 additions & 8 deletions

File tree

internal/git/diff.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ func getTrackedDiff(repoDir, baselineSHA string) ([]internal.FileDiff, error) {
6262
}
6363

6464
// getUntrackedDiff expands untracked files and directories to synthetic added diffs.
65+
// TODO(v2): reads entire file into memory; consider capping read size for large
66+
// generated files and delegating binary detection to git.
6567
func getUntrackedDiff(repoDir string) ([]internal.FileDiff, error) {
6668
cmd := exec.Command("git", "status", "--porcelain", "-z", "--untracked-files=all")
6769
cmd.Dir = repoDir

internal/ui/model.go

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
6464
m.Notification = "baseline reset"
6565
m.NewCount = 0
6666
m.NewFiles = make(map[string]bool)
67+
m.LastChangedPath = ""
6768
return m, tea.Tick(2*time.Second, func(time.Time) tea.Msg {
6869
return ClearNotificationMsg{}
6970
})
@@ -107,19 +108,38 @@ func (m Model) applyFilesUpdate(msg FilesUpdatedMsg) Model {
107108
m.Files = msg.Files
108109

109110
if m.FollowOn && len(m.Files) > 0 {
110-
target := len(m.Files) - 1
111+
// Try to follow the most recently changed file that still exists in the list.
112+
target := -1
113+
findTarget:
111114
for i := len(msg.ChangedPaths) - 1; i >= 0; i-- {
112-
changedPath := msg.ChangedPaths[i]
113115
for j, file := range m.Files {
114-
if file.Path == changedPath {
116+
if file.Path == msg.ChangedPaths[i] {
115117
target = j
116-
goto followTargetFound
118+
break findTarget
117119
}
118120
}
119121
}
120-
followTargetFound:
121-
m.CurrentIdx = target
122-
m.ScrollOffset = 0
122+
if target >= 0 {
123+
m.CurrentIdx = target
124+
m.ScrollOffset = 0
125+
} else {
126+
// No changed file matched; try to stay on the current file.
127+
anchored := false
128+
for i, file := range m.Files {
129+
if file.Path == currentPath {
130+
m.CurrentIdx = i
131+
anchored = true
132+
break
133+
}
134+
}
135+
if !anchored {
136+
// Current file disappeared; clamp to adjacent position.
137+
if m.CurrentIdx >= len(m.Files) {
138+
m.CurrentIdx = len(m.Files) - 1
139+
}
140+
m.ScrollOffset = 0
141+
}
142+
}
123143
m.NewFiles = make(map[string]bool)
124144
m.LastChangedPath = ""
125145
m.NewCount = 0

internal/ui/model_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,57 @@ func TestModelDropsQueuedOverlayUpdateAfterBaselineReset(t *testing.T) {
230230
}
231231
}
232232

233+
// TestModelFollowAnchorsWhenPrecedingFilesDisappear verifies that follow mode
234+
// keeps the current file selected when earlier files are removed from the list.
235+
func TestModelFollowAnchorsWhenPrecedingFilesDisappear(t *testing.T) {
236+
model := NewModel("repo", "/tmp/repo", "sha", []internal.FileDiff{
237+
file("a.txt", 1),
238+
file("b.txt", 1),
239+
file("c.txt", 1),
240+
file("d.txt", 1),
241+
})
242+
model.CurrentIdx = 2 // viewing c.txt
243+
244+
updated, _ := model.Update(FilesUpdatedMsg{
245+
Files: []internal.FileDiff{
246+
file("b.txt", 1),
247+
file("c.txt", 1),
248+
file("d.txt", 1),
249+
},
250+
ChangedPaths: []string{"a.txt"}, // reverted, no longer in list
251+
})
252+
253+
got := updated.(Model)
254+
if got.CurrentIdx != 1 || got.Files[got.CurrentIdx].Path != "c.txt" {
255+
t.Fatalf("CurrentIdx = %d (%s), want 1 (c.txt)", got.CurrentIdx, got.Files[got.CurrentIdx].Path)
256+
}
257+
}
258+
259+
// TestModelFollowClampsWhenCurrentFileDisappears verifies that follow mode
260+
// selects the adjacent file when the currently viewed file is reverted.
261+
func TestModelFollowClampsWhenCurrentFileDisappears(t *testing.T) {
262+
model := NewModel("repo", "/tmp/repo", "sha", []internal.FileDiff{
263+
file("a.txt", 1),
264+
file("b.txt", 1),
265+
file("c.txt", 1),
266+
})
267+
model.CurrentIdx = 2 // viewing c.txt
268+
269+
updated, _ := model.Update(FilesUpdatedMsg{
270+
Files: []internal.FileDiff{
271+
file("a.txt", 1),
272+
file("b.txt", 1),
273+
},
274+
ChangedPaths: []string{"c.txt"}, // reverted, gone
275+
})
276+
277+
got := updated.(Model)
278+
// Old CurrentIdx was 2, list now has 2 items, clamp to 1 (b.txt).
279+
if got.CurrentIdx != 1 {
280+
t.Fatalf("CurrentIdx = %d, want 1", got.CurrentIdx)
281+
}
282+
}
283+
233284
// TestModelViewSmallHeightDoesNotPanic verifies tiny terminal heights do not
234285
// trigger negative content heights in overlay rendering.
235286
func TestModelViewSmallHeightDoesNotPanic(t *testing.T) {

internal/watcher/watcher.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,8 @@ func isHeadOrRefPath(path, gitDir string) bool {
165165
}
166166

167167
// isIgnored checks whether git would ignore this path.
168+
// TODO(v2): forks a git process per event; consider caching .gitignore rules
169+
// or using an in-process ignore matcher to reduce overhead under high-frequency writes.
168170
func (fw *FileWatcher) isIgnored(path string) bool {
169171
rel := path
170172
if computed, err := filepath.Rel(fw.repoDir, path); err == nil {

main.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,5 +124,16 @@ func sendBaselineReset(stderr io.Writer, sender messageSender, root, newSHA stri
124124
// Bubble Tea preserves send order, so the reset lands before the fresh diff
125125
// recomputation for the new baseline.
126126
sender.Send(ui.BaselineResetMsg{NewSHA: newSHA})
127-
sendFilesUpdated(stderr, sender, root, newSHA, nil)
127+
128+
newFiles, computeErr := gitpkg.ComputeDiff(root, newSHA)
129+
if computeErr != nil {
130+
_, _ = fmt.Fprintf(stderr, "Error computing diff: %v\n", computeErr)
131+
// Clear stale files so the UI does not show diffs from the old baseline.
132+
sender.Send(ui.FilesUpdatedMsg{BaselineSHA: newSHA})
133+
return
134+
}
135+
sender.Send(ui.FilesUpdatedMsg{
136+
BaselineSHA: newSHA,
137+
Files: newFiles,
138+
})
128139
}

0 commit comments

Comments
 (0)