Skip to content

Commit 93de374

Browse files
committed
fix insert branch bugs in modify TUI
Fix three bugs with the insert branch feature in the modify TUI, and adjust rename behavior on inserted nodes. ## Bug 1: False "moved" annotations on existing branches After inserting a branch, all branches below the insertion point displayed "↕ moved 1 layer down" annotations. This happened because `nodeAnnotation` and `toNodeData` compared each node's `OriginalPosition` against its raw array index, which gets shifted when an inserted node is added to the slice. Fix: introduce an `effectiveIdx` parameter that counts only non-inserted nodes, so position comparisons reflect the original ordering. The View loop computes effective indices by incrementing only for non-inserted nodes and passes them to the rendering functions. ## Bug 2: Header branch count inflated by staged inserts The branch count in the header ("N branches") included inserted placeholder nodes, making it appear as though the stack had grown before changes were applied. Fix: `buildHeaderConfig` now excludes `IsInserted` nodes from the branch count. The count reflects only the original branches in the stack. ## Bug 3: Operations allowed on inserted placeholder nodes Inserted nodes could be folded into other branches, which makes no sense for a placeholder with no commits. Additionally, the "last branch" guard counted inserted nodes as active, allowing users to drop all original branches and bypass the empty-stack check. Fix: - `fold()` rejects inserted nodes with a descriptive error message. - `toggleDrop()` on an inserted node removes it entirely and pops the original insert action from the undo stack (clean cancellation rather than a separate undo entry). - All three "active branch" guards (`toggleDrop`, `fold`, `tryApply`) now exclude `IsInserted` nodes, ensuring at least one original branch always remains in the stack. ## Rename on inserted branches Instead of blocking renames on inserted nodes, pressing `r` now enters rename mode and updates the insert action's name in place. The node's `Ref.Branch` and `PendingAction.NewName` are both updated directly — no separate rename action is created in the undo stack. This lets users fix a typo without having to drop and re-insert. ## Tests added - `TestInsertDoesNotShowMovedAnnotation` — verifies no false move annotations appear on existing branches after an insert - `TestBranchCountExcludesInserts` — verifies header count stays stable after insert - `TestCannotFoldInsertedBranch` — verifies fold is blocked - `TestCannotRenameInsertedBranch` — verifies rename updates the insert name in place - `TestDropInsertedBranchRemovesIt` — verifies drop removes the node - `TestDropInsertedBranchCanBeUndone` — verifies drop pops the original insert from the undo stack - `TestCannotDropAllOriginalBranchesWithInsert` — verifies the empty-stack guard excludes inserted nodes
1 parent d7d2bb9 commit 93de374

2 files changed

Lines changed: 272 additions & 20 deletions

File tree

internal/tui/modifyview/model.go

Lines changed: 82 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,24 @@ func (m Model) updateRename(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
313313
}
314314
}
315315

316+
// For inserted nodes, update the insert action's name directly
317+
// rather than creating a separate rename action.
318+
if node.IsInserted {
319+
oldInsertName := node.Ref.Branch
320+
node.Ref.Branch = newName
321+
node.PendingAction.NewName = newName
322+
// Update the matching undo stack entry
323+
for i := len(m.actionStack) - 1; i >= 0; i-- {
324+
a := &m.actionStack[i]
325+
if (a.Type == ActionInsertBelow || a.Type == ActionInsertAbove) && a.BranchName == oldInsertName {
326+
a.BranchName = newName
327+
break
328+
}
329+
}
330+
m.renameMode = false
331+
return m, nil
332+
}
333+
316334
// Record undo action
317335
m.actionStack = append(m.actionStack, StagedAction{
318336
Type: ActionRename,
@@ -443,7 +461,7 @@ func (m Model) updateNormal(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
443461

444462
case key.Matches(msg, modifyKeys.MoveUp):
445463
if m.currentMode() == modeStructure {
446-
m.statusMessage = "Cannot reorder while drops, folds, or renames are staged — undo them first"
464+
m.statusMessage = "Cannot reorder while drops, folds, inserts, or renames are staged — undo them first"
447465
m.statusIsError = true
448466
return m, nil
449467
}
@@ -452,7 +470,7 @@ func (m Model) updateNormal(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
452470

453471
case key.Matches(msg, modifyKeys.MoveDown):
454472
if m.currentMode() == modeStructure {
455-
m.statusMessage = "Cannot reorder while drops, folds, or renames are staged — undo them first"
473+
m.statusMessage = "Cannot reorder while drops, folds, inserts, or renames are staged — undo them first"
456474
m.statusIsError = true
457475
return m, nil
458476
}
@@ -664,6 +682,30 @@ func (m *Model) toggleDrop() {
664682
return
665683
}
666684

685+
// Dropping an inserted node removes it entirely (undo the insert).
686+
// Pop the original insert action from the undo stack rather than
687+
// pushing a new entry — this makes the drop behave as a direct
688+
// cancellation of the insert.
689+
if node.IsInserted {
690+
branchName := node.Ref.Branch
691+
m.nodes = append(m.nodes[:m.cursor], m.nodes[m.cursor+1:]...)
692+
// Remove the matching insert action from the undo stack
693+
for i := len(m.actionStack) - 1; i >= 0; i-- {
694+
a := m.actionStack[i]
695+
if (a.Type == ActionInsertBelow || a.Type == ActionInsertAbove) && a.BranchName == branchName {
696+
m.actionStack = append(m.actionStack[:i], m.actionStack[i+1:]...)
697+
break
698+
}
699+
}
700+
if m.cursor >= len(m.nodes) {
701+
m.cursor = len(m.nodes) - 1
702+
}
703+
if m.cursor < 0 {
704+
m.cursor = 0
705+
}
706+
return
707+
}
708+
667709
if node.PendingAction != nil && node.PendingAction.Type == ActionDrop {
668710
// Undo drop
669711
m.actionStack = append(m.actionStack, StagedAction{
@@ -708,13 +750,13 @@ func (m *Model) toggleDrop() {
708750
}
709751
}
710752

711-
// Check if this would remove the last active branch
753+
// Check if this would remove the last active original branch
712754
active := 0
713755
for j, other := range m.nodes {
714756
if j == m.cursor {
715757
continue // skip the branch we're about to drop
716758
}
717-
if !other.Removed && !other.Ref.IsMerged() {
759+
if !other.Removed && !other.Ref.IsMerged() && !other.IsInserted {
718760
active++
719761
}
720762
}
@@ -740,6 +782,11 @@ func (m *Model) fold(action ActionType) {
740782
return
741783
}
742784
node := &m.nodes[m.cursor]
785+
if node.IsInserted {
786+
m.statusMessage = "Cannot fold an inserted branch — drop it with x to remove"
787+
m.statusIsError = true
788+
return
789+
}
743790
if node.Ref.IsMerged() {
744791
m.statusMessage = "Cannot fold a merged branch"
745792
m.statusIsError = true
@@ -803,13 +850,13 @@ func (m *Model) fold(action ActionType) {
803850
}
804851
}
805852

806-
// Check if this would remove the last active branch
853+
// Check if this would remove the last active original branch
807854
active := 0
808855
for j, other := range m.nodes {
809856
if j == m.cursor {
810857
continue
811858
}
812-
if !other.Removed && !other.Ref.IsMerged() {
859+
if !other.Removed && !other.Ref.IsMerged() && !other.IsInserted {
813860
active++
814861
}
815862
}
@@ -979,10 +1026,10 @@ func (m Model) tryApply() (tea.Model, tea.Cmd) {
9791026
return m, nil
9801027
}
9811028

982-
// Ensure at least one non-removed, non-merged branch remains
1029+
// Ensure at least one non-removed, non-merged, non-inserted branch remains
9831030
active := 0
9841031
for _, n := range m.nodes {
985-
if !n.Removed && !n.Ref.IsMerged() {
1032+
if !n.Removed && !n.Ref.IsMerged() && !n.IsInserted {
9861033
active++
9871034
}
9881035
}
@@ -1013,7 +1060,7 @@ func (m *Model) ensureVisible() {
10131060
}
10141061

10151062
func (m Model) nodeLineCount(idx int) int {
1016-
return shared.NodeLineCount(toNodeData(m.nodes[idx], idx))
1063+
return shared.NodeLineCount(toNodeData(m.nodes[idx], idx, idx))
10171064
}
10181065

10191066
func (m Model) contentViewHeight() int {
@@ -1043,7 +1090,7 @@ func (m *Model) clampScroll() {
10431090
func (m Model) handleMouseClick(screenX, screenY int) (tea.Model, tea.Cmd) {
10441091
nodes := make([]shared.BranchNodeData, len(m.nodes))
10451092
for i, n := range m.nodes {
1046-
nodes[i] = toNodeData(n, i)
1093+
nodes[i] = toNodeData(n, i, i)
10471094
}
10481095

10491096
result := shared.HandleClick(screenX, screenY, nodes, m.width, m.height, m.scrollOffset, shared.ShouldShowHeader(m.width, m.height), false)
@@ -1077,8 +1124,9 @@ func (m Model) handleMouseClick(screenX, screenY int) (tea.Model, tea.Cmd) {
10771124

10781125
// toNodeData converts a ModifyBranchNode to shared.BranchNodeData,
10791126
// applying drop/fold/move visual overrides. currentIdx is the node's
1080-
// current position in the list, used to detect moves.
1081-
func toNodeData(n ModifyBranchNode, currentIdx int) shared.BranchNodeData {
1127+
// current position in the list. effectiveIdx is the position among
1128+
// non-inserted nodes (used for move detection).
1129+
func toNodeData(n ModifyBranchNode, currentIdx int, effectiveIdx int) shared.BranchNodeData {
10821130
data := shared.BranchNodeData{
10831131
Ref: n.Ref,
10841132
IsCurrent: n.IsCurrent,
@@ -1116,7 +1164,7 @@ func toNodeData(n ModifyBranchNode, currentIdx int) shared.BranchNodeData {
11161164
}
11171165

11181166
// Moved branch: purple solid connector (no dash, no strikethrough)
1119-
if n.PendingAction == nil && !n.Ref.IsMerged() && !n.IsInserted && n.OriginalPosition != currentIdx {
1167+
if n.PendingAction == nil && !n.Ref.IsMerged() && !n.IsInserted && n.OriginalPosition != effectiveIdx {
11201168
c := movedConnectorStyle
11211169
data.ConnectorStyleOverride = &c
11221170
}
@@ -1125,8 +1173,9 @@ func toNodeData(n ModifyBranchNode, currentIdx int) shared.BranchNodeData {
11251173
}
11261174

11271175
// nodeAnnotation builds an optional annotation from the node's pending action
1128-
// or its position change. currentIdx is the node's current position in the list.
1129-
func nodeAnnotation(n ModifyBranchNode, currentIdx int) *shared.NodeAnnotation {
1176+
// or its position change. effectiveIdx is the node's position among non-inserted
1177+
// nodes, used for move detection.
1178+
func nodeAnnotation(n ModifyBranchNode, effectiveIdx int) *shared.NodeAnnotation {
11301179
if n.Ref.IsMerged() {
11311180
return &shared.NodeAnnotation{Text: "🔒", Style: shared.DimStyle}
11321181
}
@@ -1147,8 +1196,8 @@ func nodeAnnotation(n ModifyBranchNode, currentIdx int) *shared.NodeAnnotation {
11471196
}
11481197
}
11491198
// Show move annotation when position changed (even without explicit PendingAction)
1150-
if !n.Ref.IsMerged() && !n.IsInserted && n.OriginalPosition != currentIdx {
1151-
delta := n.OriginalPosition - currentIdx // positive = moved up (toward top)
1199+
if !n.Ref.IsMerged() && !n.IsInserted && n.OriginalPosition != effectiveIdx {
1200+
delta := n.OriginalPosition - effectiveIdx // positive = moved up (toward top)
11521201
direction := "up"
11531202
layers := delta
11541203
if delta < 0 {
@@ -1184,10 +1233,17 @@ func (m Model) View() string {
11841233

11851234
// Build the scrollable branch list content
11861235
var b strings.Builder
1236+
effectiveIdx := 0
11871237
for i := 0; i < len(m.nodes); i++ {
1188-
nodeData := toNodeData(m.nodes[i], i)
1238+
ei := effectiveIdx
1239+
if m.nodes[i].IsInserted {
1240+
ei = -1 // inserted nodes have no effective position
1241+
} else {
1242+
effectiveIdx++
1243+
}
1244+
nodeData := toNodeData(m.nodes[i], i, ei)
11891245
isFocused := i == m.cursor
1190-
annotation := nodeAnnotation(m.nodes[i], i)
1246+
annotation := nodeAnnotation(m.nodes[i], ei)
11911247
shared.RenderNode(&b, nodeData, isFocused, m.width, annotation)
11921248
}
11931249
shared.RenderTrunk(&b, m.trunk.Branch)
@@ -1247,7 +1303,13 @@ func (m Model) buildHeaderConfig() shared.HeaderConfig {
12471303
}
12481304
}
12491305

1250-
branchCount := len(m.nodes)
1306+
// Count only original branches (exclude staged inserts)
1307+
branchCount := 0
1308+
for _, n := range m.nodes {
1309+
if !n.IsInserted {
1310+
branchCount++
1311+
}
1312+
}
12511313
branchInfo := fmt.Sprintf("%d branches", branchCount)
12521314
if branchCount == 1 {
12531315
branchInfo = "1 branch"

0 commit comments

Comments
 (0)