Skip to content

Commit 691f469

Browse files
committed
disable selecting merged branches in TUIs
1 parent c9d9ad6 commit 691f469

4 files changed

Lines changed: 111 additions & 15 deletions

File tree

internal/tui/modifyview/model.go

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -454,12 +454,16 @@ func (m *Model) currentMode() actionMode {
454454
return modeNone
455455
}
456456

457-
// moveCursor moves the cursor by delta to the next node.
457+
// moveCursor moves the cursor by delta, skipping merged branches.
458458
func (m *Model) moveCursor(delta int) {
459459
next := m.cursor + delta
460-
if next >= 0 && next < len(m.nodes) {
461-
m.cursor = next
462-
m.ensureVisible()
460+
for next >= 0 && next < len(m.nodes) {
461+
if !m.nodes[next].Ref.IsMerged() {
462+
m.cursor = next
463+
m.ensureVisible()
464+
return
465+
}
466+
next += delta
463467
}
464468
}
465469

@@ -866,6 +870,11 @@ func (m Model) handleMouseClick(screenX, screenY int) (tea.Model, tea.Cmd) {
866870
return m, nil
867871
}
868872

873+
// Don't allow selecting merged branches.
874+
if m.nodes[result.NodeIndex].Ref.IsMerged() {
875+
return m, nil
876+
}
877+
869878
m.cursor = result.NodeIndex
870879

871880
if result.OpenURL != "" {

internal/tui/modifyview/model_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -756,3 +756,21 @@ func TestUndoRename_DoesNotAffectOtherRenames(t *testing.T) {
756756
require.NotNil(t, m.nodes[0].PendingAction, "first rename should still be intact")
757757
assert.Equal(t, "a-renamed", m.nodes[0].PendingAction.NewName, "first rename new name should be unchanged")
758758
}
759+
760+
func TestCursorNavigation_SkipsMergedBranches(t *testing.T) {
761+
nodes := []ModifyBranchNode{
762+
makeNode("a", true, 0),
763+
makeMergedNode("merged", 1),
764+
makeNode("c", false, 2),
765+
}
766+
m := New(nodes, testTrunk, "1.0.0")
767+
require.Equal(t, 0, m.cursor, "cursor should start on first non-merged")
768+
769+
// Move down should skip merged and land on c
770+
m = sendKey(t, m, runeKey('j'))
771+
assert.Equal(t, 2, m.cursor, "down should skip merged branch")
772+
773+
// Move up should skip merged and land back on a
774+
m = sendKey(t, m, runeKey('k'))
775+
assert.Equal(t, 0, m.cursor, "up should skip merged branch")
776+
}

internal/tui/stackview/model.go

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,24 @@ func New(nodes []BranchNode, trunk stack.BranchRef, version string) Model {
8383
h := help.New()
8484
h.ShowAll = true
8585

86-
// Cursor starts at the current branch, or top of stack
86+
// Cursor starts at the current branch, or first non-merged branch
8787
cursor := 0
88+
found := false
8889
for i, n := range nodes {
89-
if n.IsCurrent {
90+
if n.IsCurrent && !n.Ref.IsMerged() {
9091
cursor = i
92+
found = true
9193
break
9294
}
9395
}
96+
if !found {
97+
for i, n := range nodes {
98+
if !n.Ref.IsMerged() {
99+
cursor = i
100+
break
101+
}
102+
}
103+
}
94104

95105
return Model{
96106
nodes: nodes,
@@ -124,17 +134,11 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
124134
return m, tea.Quit
125135

126136
case key.Matches(msg, keys.Up):
127-
if m.cursor > 0 {
128-
m.cursor--
129-
m.ensureVisible()
130-
}
137+
m.moveCursor(-1)
131138
return m, nil
132139

133140
case key.Matches(msg, keys.Down):
134-
if m.cursor < len(m.nodes)-1 {
135-
m.cursor++
136-
m.ensureVisible()
137-
}
141+
m.moveCursor(1)
138142
return m, nil
139143

140144
case key.Matches(msg, keys.ToggleCommits):
@@ -165,7 +169,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
165169
case key.Matches(msg, keys.Checkout):
166170
if m.cursor >= 0 && m.cursor < len(m.nodes) {
167171
node := m.nodes[m.cursor]
168-
if !node.IsCurrent {
172+
if !node.IsCurrent && !node.Ref.IsMerged() {
169173
m.checkoutBranch = node.Ref.Branch
170174
return m, tea.Quit
171175
}
@@ -226,6 +230,11 @@ func (m Model) handleMouseClick(screenX, screenY int) (tea.Model, tea.Cmd) {
226230
return m, nil
227231
}
228232

233+
// Don't allow selecting merged branches.
234+
if m.nodes[result.NodeIndex].Ref.IsMerged() {
235+
return m, nil
236+
}
237+
229238
m.cursor = result.NodeIndex
230239

231240
if result.OpenURL != "" {
@@ -248,6 +257,19 @@ func (m Model) nodeLineCount(idx int) int {
248257
return shared.NodeLineCount(toBranchNodeData(m.nodes[idx]))
249258
}
250259

260+
// moveCursor moves the cursor by delta, skipping merged branches.
261+
func (m *Model) moveCursor(delta int) {
262+
next := m.cursor + delta
263+
for next >= 0 && next < len(m.nodes) {
264+
if !m.nodes[next].Ref.IsMerged() {
265+
m.cursor = next
266+
m.ensureVisible()
267+
return
268+
}
269+
next += delta
270+
}
271+
}
272+
251273
// ensureVisible adjusts scroll offset so the cursor is visible.
252274
func (m *Model) ensureVisible() {
253275
if m.height == 0 {

internal/tui/stackview/model_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,3 +341,50 @@ func TestScrollClamp_CannotScrollPastContent(t *testing.T) {
341341
view := m.View()
342342
assert.Contains(t, view, "b1", "content should still be visible after excessive scrolling")
343343
}
344+
345+
func TestUpdate_CursorSkipsMergedBranches(t *testing.T) {
346+
nodes := makeNodes("b1", "b2", "b3")
347+
nodes[1].Ref.PullRequest = &stack.PullRequestRef{Number: 2, Merged: true}
348+
m := New(nodes, testTrunk, "0.0.1")
349+
assert.Equal(t, 0, m.cursor, "cursor should start on first non-merged branch")
350+
351+
// Down should skip b2 (merged) and land on b3
352+
updated, _ := m.Update(keyMsg("down"))
353+
m = updated.(Model)
354+
assert.Equal(t, 2, m.cursor, "down should skip merged b2 and land on b3")
355+
356+
// Up should skip b2 (merged) and land back on b1
357+
updated, _ = m.Update(keyMsg("up"))
358+
m = updated.(Model)
359+
assert.Equal(t, 0, m.cursor, "up should skip merged b2 and land on b1")
360+
}
361+
362+
func TestNew_CursorSkipsMergedBranch(t *testing.T) {
363+
nodes := makeNodes("b1", "b2", "b3")
364+
nodes[0].Ref.PullRequest = &stack.PullRequestRef{Number: 1, Merged: true}
365+
m := New(nodes, testTrunk, "0.0.1")
366+
assert.Equal(t, 1, m.cursor, "cursor should skip merged b1 and start on b2")
367+
}
368+
369+
func TestNew_CursorSkipsMergedCurrentBranch(t *testing.T) {
370+
nodes := makeNodes("b1", "b2", "b3")
371+
nodes[0].IsCurrent = true
372+
nodes[0].Ref.PullRequest = &stack.PullRequestRef{Number: 1, Merged: true}
373+
m := New(nodes, testTrunk, "0.0.1")
374+
assert.Equal(t, 1, m.cursor, "cursor should not start on merged current branch")
375+
}
376+
377+
func TestUpdate_EnterOnMergedDoesNothing(t *testing.T) {
378+
// All non-merged so we can navigate, but force cursor onto a merged node
379+
// by having b1 active and b2 merged and b3 active.
380+
nodes := makeNodes("b1", "b2")
381+
nodes[0].Ref.PullRequest = &stack.PullRequestRef{Number: 1, Merged: true}
382+
m := New(nodes, testTrunk, "0.0.1")
383+
// Cursor is on b2 (first non-merged). Manually set to b1 to test guard.
384+
m.cursor = 0
385+
386+
updated, cmd := m.Update(keyMsg("enter"))
387+
m = updated.(Model)
388+
assert.Equal(t, "", m.CheckoutBranch(), "enter on merged branch should not set checkout")
389+
assert.Nil(t, cmd, "enter on merged branch should not quit")
390+
}

0 commit comments

Comments
 (0)