From ac16ab32d88d89e170aa61fe84654153f73dc69c Mon Sep 17 00:00:00 2001 From: Jan Smrcka Date: Mon, 23 Feb 2026 14:32:28 +0100 Subject: [PATCH 1/4] fix: preserve diff scroll position during auto-refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 2s poll tick was resetting viewport to top via unconditional GotoTop() in handleDiffLoaded. Add resetScroll flag to diffLoadedMsg — only true for user-initiated loads (file switch, split toggle, resize). Tick refreshes pass false. Also skip viewport update entirely when content unchanged (lastDiffContent dedup). Co-Authored-By: Claude Opus 4.6 --- internal/ui/model.go | 43 ++++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/internal/ui/model.go b/internal/ui/model.go index c984a27..64191a8 100644 --- a/internal/ui/model.go +++ b/internal/ui/model.go @@ -32,8 +32,9 @@ type tickMsg time.Time // Messages type diffLoadedMsg struct { - content string - index int + content string + index int + resetScroll bool } type filesRefreshedMsg struct { @@ -94,6 +95,8 @@ type Model struct { ready bool SelectedFile string // set on "open in editor" action, read after Run() + lastDiffContent string + // Branch picker state branches []string branchCursor int @@ -192,7 +195,7 @@ func (m *Model) StartInCommitMode() { } func (m Model) Init() tea.Cmd { - cmds := []tea.Cmd{m.loadDiffCmd(), m.fetchUpstreamStatusCmd(), tickCmd()} + cmds := []tea.Cmd{m.loadDiffCmd(true), m.fetchUpstreamStatusCmd(), tickCmd()} if m.mode == modeCommit { cmds = append(cmds, textinput.Blink) } @@ -250,12 +253,19 @@ func (m Model) handleResize(msg tea.WindowSizeMsg) (tea.Model, tea.Cmd) { m.height = msg.Height m.viewport = viewport.New(m.diffWidth(), m.contentHeight()) m.ready = true - return m, m.loadDiffCmd() + return m, m.loadDiffCmd(true) } func (m Model) handleDiffLoaded(msg diffLoadedMsg) (tea.Model, tea.Cmd) { - if msg.index == m.cursor { - m.viewport.SetContent(msg.content) + if msg.index != m.cursor { + return m, nil + } + if msg.content == m.lastDiffContent { + return m, nil + } + m.lastDiffContent = msg.content + m.viewport.SetContent(msg.content) + if msg.resetScroll { m.viewport.GotoTop() } return m, nil @@ -263,18 +273,19 @@ func (m Model) handleDiffLoaded(msg diffLoadedMsg) (tea.Model, tea.Cmd) { func (m Model) handleFilesRefreshed(msg filesRefreshedMsg) (tea.Model, tea.Cmd) { if filesEqual(m.files, msg.files) { - return m, m.loadDiffCmd() + return m, m.loadDiffCmd(false) } m.files = msg.files if m.cursor >= len(m.files) { m.cursor = max(0, len(m.files)-1) } m.prevCurs = -1 + m.lastDiffContent = "" if len(m.files) == 0 { m.viewport.SetContent("") return m, nil } - return m, m.loadDiffCmd() + return m, m.loadDiffCmd(true) } func (m Model) handleCommitDone(msg commitDoneMsg) (tea.Model, tea.Cmd) { @@ -453,7 +464,8 @@ func (m Model) updateFileListMode(msg tea.KeyMsg) (tea.Model, tea.Cmd) { case "v": m.splitDiff = !m.splitDiff m.prevCurs = -1 - return m, tea.Batch(m.loadDiffCmd(), m.saveSplitPrefCmd()) + m.lastDiffContent = "" + return m, tea.Batch(m.loadDiffCmd(true), m.saveSplitPrefCmd()) case "F": if m.upstream.Upstream == "" { m.statusMsg = "no upstream configured" @@ -464,7 +476,7 @@ func (m Model) updateFileListMode(msg tea.KeyMsg) (tea.Model, tea.Cmd) { } if m.cursor != m.prevCurs { m.prevCurs = m.cursor - return m, m.loadDiffCmd() + return m, m.loadDiffCmd(true) } return m, nil } @@ -493,7 +505,8 @@ func (m Model) updateDiffMode(msg tea.KeyMsg) (tea.Model, tea.Cmd) { case "v": m.splitDiff = !m.splitDiff m.prevCurs = -1 - return m, tea.Batch(m.loadDiffCmd(), m.saveSplitPrefCmd()) + m.lastDiffContent = "" + return m, tea.Batch(m.loadDiffCmd(true), m.saveSplitPrefCmd()) } var cmd tea.Cmd m.viewport, cmd = m.viewport.Update(msg) @@ -623,7 +636,7 @@ func (m Model) nextFile() (tea.Model, tea.Cmd) { if m.cursor < len(m.files)-1 { m.cursor++ m.prevCurs = m.cursor - return m, m.loadDiffCmd() + return m, m.loadDiffCmd(true) } return m, nil } @@ -632,13 +645,13 @@ func (m Model) prevFile() (tea.Model, tea.Cmd) { if m.cursor > 0 { m.cursor-- m.prevCurs = m.cursor - return m, m.loadDiffCmd() + return m, m.loadDiffCmd(true) } return m, nil } // Commands -func (m Model) loadDiffCmd() tea.Cmd { +func (m Model) loadDiffCmd(resetScroll bool) tea.Cmd { if len(m.files) == 0 { return nil } @@ -677,7 +690,7 @@ func (m Model) loadDiffCmd() tea.Cmd { } } } - return diffLoadedMsg{content: content, index: idx} + return diffLoadedMsg{content: content, index: idx, resetScroll: resetScroll} } } From 3bc5e0595fe3d332185353f776cc3988c3d8975a Mon Sep 17 00:00:00 2001 From: Jan Smrcka Date: Mon, 23 Feb 2026 14:36:20 +0100 Subject: [PATCH 2/4] fix: clear diff cache on resize to prevent empty viewport viewport.New() creates empty viewport, but dedup check in handleDiffLoaded skipped SetContent when diff unchanged. Co-Authored-By: Claude Opus 4.6 --- internal/ui/model.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/ui/model.go b/internal/ui/model.go index 64191a8..0c18bd6 100644 --- a/internal/ui/model.go +++ b/internal/ui/model.go @@ -252,6 +252,7 @@ func (m Model) handleResize(msg tea.WindowSizeMsg) (tea.Model, tea.Cmd) { m.width = msg.Width m.height = msg.Height m.viewport = viewport.New(m.diffWidth(), m.contentHeight()) + m.lastDiffContent = "" // force re-apply after viewport recreation m.ready = true return m, m.loadDiffCmd(true) } From 6d35108e7266a4cff60454265a20804206559828 Mon Sep 17 00:00:00 2001 From: Jan Smrcka Date: Mon, 23 Feb 2026 14:44:58 +0100 Subject: [PATCH 3/4] test: add regression tests for resize diff cache clearing Locks behavior: handleResize clears lastDiffContent so handleDiffLoaded re-applies content to recreated viewport. Co-Authored-By: Claude Opus 4.6 --- internal/ui/model_test.go | 43 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/internal/ui/model_test.go b/internal/ui/model_test.go index 297a497..54375bb 100644 --- a/internal/ui/model_test.go +++ b/internal/ui/model_test.go @@ -317,6 +317,49 @@ func TestHandleBranchesLoaded_Error(t *testing.T) { } } +func TestHandleResize_ClearsDiffCache(t *testing.T) { + t.Parallel() + m := newTestModel(t, []fileItem{ + {change: git.FileChange{Path: "a.go", Status: git.StatusModified}}, + }) + m.cursor = 0 + + // Simulate having cached diff content + m.lastDiffContent = "old diff" + m.viewport.SetContent("old diff") + + // Resize creates new viewport — cache must be cleared + result, _ := m.handleResize(tea.WindowSizeMsg{Width: 100, Height: 40}) + rm := result.(Model) + + if rm.lastDiffContent != "" { + t.Error("handleResize should clear lastDiffContent to force re-apply") + } + + // Now handleDiffLoaded with same content should apply (not skip) + result2, _ := rm.handleDiffLoaded(diffLoadedMsg{content: "old diff", index: 0}) + rm2 := result2.(Model) + if rm2.lastDiffContent != "old diff" { + t.Error("handleDiffLoaded should apply content after resize cleared cache") + } +} + +func TestHandleDiffLoaded_SkipsDuplicate(t *testing.T) { + t.Parallel() + m := newTestModel(t, []fileItem{ + {change: git.FileChange{Path: "a.go", Status: git.StatusModified}}, + }) + m.cursor = 0 + m.lastDiffContent = "same diff" + + // Same content as cache — should be a no-op + result, _ := m.handleDiffLoaded(diffLoadedMsg{content: "same diff", index: 0}) + rm := result.(Model) + if rm.lastDiffContent != "same diff" { + t.Error("cache should remain unchanged on duplicate") + } +} + func TestBranchListScroll(t *testing.T) { t.Parallel() m := newTestModel(t, nil) From 131bb55ff5a773c8db2547f9a11ed899df02a841 Mon Sep 17 00:00:00 2001 From: Jan Smrcka Date: Mon, 23 Feb 2026 14:47:29 +0100 Subject: [PATCH 4/4] test: assert viewport content directly in resize regression test Co-Authored-By: Claude Opus 4.6 --- internal/ui/model_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/ui/model_test.go b/internal/ui/model_test.go index 54375bb..421d219 100644 --- a/internal/ui/model_test.go +++ b/internal/ui/model_test.go @@ -336,12 +336,15 @@ func TestHandleResize_ClearsDiffCache(t *testing.T) { t.Error("handleResize should clear lastDiffContent to force re-apply") } - // Now handleDiffLoaded with same content should apply (not skip) + // handleDiffLoaded with same content should apply (not skip) after resize result2, _ := rm.handleDiffLoaded(diffLoadedMsg{content: "old diff", index: 0}) rm2 := result2.(Model) if rm2.lastDiffContent != "old diff" { t.Error("handleDiffLoaded should apply content after resize cleared cache") } + if !strings.Contains(rm2.viewport.View(), "old diff") { + t.Errorf("viewport should contain reapplied content, got %q", rm2.viewport.View()) + } } func TestHandleDiffLoaded_SkipsDuplicate(t *testing.T) {