diff --git a/README.md b/README.md index 3131de0..bd7489f 100644 --- a/README.md +++ b/README.md @@ -245,7 +245,7 @@ Interactively restructure the current stack. gh stack modify [flags] ``` -Opens a terminal UI for restructuring a stack. You can rename, drop, reorder, and fold branches into adjacent ones. All the changes are staged during the preview and applied at once on save. +Opens a terminal UI for restructuring a stack. You can drop, fold, insert, rename, and reorder branches. All the changes are staged during the preview and applied at once on save. If the stack of PRs has been created on GitHub, run `gh stack submit` afterwards to push the changes and recreate the stack. @@ -259,6 +259,7 @@ If the stack of PRs has been created on GitHub, run `gh stack submit` afterwards - **Drop** (`x`): Remove a branch and its commits from the stack. Local branch and associated PR are preserved. - **Fold down** (`d`): Absorb a branch's commits into the branch below (toward trunk). Folded branch removed from stack. - **Fold up** (`u`): Absorb a branch's commits into the branch above (away from trunk). Folded branch removed from stack. +- **Insert** (`i`/`I`): Insert a new empty branch into the stack. `i` inserts below the cursor; `I` inserts above. - **Reorder** (`Shift+↑`/`Shift+↓`): Move a branch up (away from trunk) or down (toward trunk) in the stack. - **Rename** (`r`): Rename a branch locally and in the stack metadata. - **Undo** (`z`): Undo the last staged action. @@ -272,7 +273,8 @@ If the stack of PRs has been created on GitHub, run `gh stack submit` afterwards | `c` | View commits | | `x` | Drop branch | | `r` | Rename branch | -| `u/d` | Fold branch up/down | +| `i/I` | Insert branch below/above | +| `d/u` | Fold branch down/up | | `Shift+↑`/`Shift+↓` | Move branch up/down | | `z` | Undo last action | | `Ctrl+S` | Apply all changes | diff --git a/cmd/add.go b/cmd/add.go index 8dcdffc..b27555b 100644 --- a/cmd/add.go +++ b/cmd/add.go @@ -85,7 +85,8 @@ func runAdd(cfg *config.Config, opts *addOptions, args []string) error { // idx < 0 means we're on the trunk — that's allowed (we'll create // a new branch from it). Only block if we're in the middle of the stack. if idx >= 0 && idx < len(s.Branches)-1 { - cfg.Errorf("can only add branches on top of the stack; run `%s` to switch to %q", cfg.ColorCyan("gh stack top"), s.Branches[len(s.Branches)-1].Branch) + cfg.Errorf("can only add branches to the top of the stack; run `%s` then `%s`", cfg.ColorCyan("gh stack top"), cfg.ColorCyan("gh stack add")) + cfg.Printf("Or to restructure your stack and insert a branch, use `%s`", cfg.ColorCyan("gh stack modify")) return ErrInvalidArgs } diff --git a/cmd/add_test.go b/cmd/add_test.go index b6f587f..8563cc1 100644 --- a/cmd/add_test.go +++ b/cmd/add_test.go @@ -78,6 +78,7 @@ func TestAdd_OnlyAllowedOnTopOfStack(t *testing.T) { output := collectOutput(cfg, outR, errR) assert.Contains(t, output, "top of the stack") + assert.Contains(t, output, "gh stack modify") } func TestAdd_MutuallyExclusiveFlags(t *testing.T) { diff --git a/cmd/modify.go b/cmd/modify.go index 3842e38..8da6e0a 100644 --- a/cmd/modify.go +++ b/cmd/modify.go @@ -29,6 +29,7 @@ func ModifyCmd(cfg *config.Config) *cobra.Command { Operations available: • Drop branches from the stack • Fold branches into adjacent branches + • Insert new branches into the stack • Reorder branches • Rename branches @@ -168,6 +169,10 @@ func printModifySuccess(cfg *config.Config, result *modifyview.ApplyResult) { cfg.Printf(" Renamed: %s → %s", r.OldName, r.NewName) } + for _, name := range result.InsertedBranches { + cfg.Printf(" Inserted: %s", name) + } + for _, d := range result.DroppedPRs { cfg.Printf(" Dropped: %s (PR #%d remains open — close with `%s`)", d.Branch, d.PRNumber, cfg.ColorCyan(fmt.Sprintf("gh pr close %d", d.PRNumber))) diff --git a/docs/src/assets/screenshots/modify-stack-tui.png b/docs/src/assets/screenshots/modify-stack-tui.png index 2c631c5..b13b870 100644 Binary files a/docs/src/assets/screenshots/modify-stack-tui.png and b/docs/src/assets/screenshots/modify-stack-tui.png differ diff --git a/docs/src/content/docs/faq.md b/docs/src/content/docs/faq.md index 6a587bd..e047da1 100644 --- a/docs/src/content/docs/faq.md +++ b/docs/src/content/docs/faq.md @@ -33,7 +33,7 @@ You can also add PRs to an existing stack from the GitHub UI. See [Adding to an ### How can I modify my stack? -Use `gh stack modify` to restructure a stack. It opens an interactive terminal UI where you can reorder, drop, fold (combine), and rename branches — then applies all changes at once. See the [Restructuring Stacks](/gh-stack/guides/modify/) guide for a full walkthrough. +Use `gh stack modify` to restructure a stack. It opens an interactive terminal UI where you can reorder, drop, fold (combine), insert, and rename branches — then applies all changes at once. See the [Restructuring Stacks](/gh-stack/guides/modify/) guide for a full walkthrough. Alternatively, you can manually tear down and re-create the stack with `gh stack unstack` and `gh stack init`: @@ -41,7 +41,7 @@ Alternatively, you can manually tear down and re-create the stack with `gh stack # 1. Remove the stack gh stack unstack -# 2. Make structural changes (reorder, rename, delete branches) +# 2. Make structural changes (reorder, rename, insert, delete branches) git branch -m api-roots api-routes # 3. Re-create the stack with the new structure diff --git a/docs/src/content/docs/guides/modify.md b/docs/src/content/docs/guides/modify.md index fd15375..0eadeca 100644 --- a/docs/src/content/docs/guides/modify.md +++ b/docs/src/content/docs/guides/modify.md @@ -3,7 +3,7 @@ title: Restructuring Stacks description: How to use `gh stack modify` to restructure a stack. --- -`gh stack modify` provides an interactive terminal UI for restructuring a stack locally. You can drop, fold, rename, and reorder branches and then apply all your changes at once. +`gh stack modify` provides an interactive terminal UI for restructuring a stack locally. You can drop, fold, insert, rename, and reorder branches and then apply all your changes at once. ![The modify stack terminal UI](../../../assets/screenshots/modify-stack-tui.png) @@ -12,6 +12,7 @@ description: How to use `gh stack modify` to restructure a stack. Use `modify` when you need to: - **Remove** a branch from the stack - **Combine** two branches into one +- **Insert** a new branch into the stack - **Rename** a branch - **Reorder** branches @@ -46,13 +47,17 @@ Absorbs the selected branch's commits into the branch below it (toward trunk) vi Absorbs the selected branch's commits into the branch above it (away from trunk). Since the branch above already contains the folded branch's commits in its history, this is handled by adjusting what is considered the first unique commit for the branch. The folded branch is removed from the stack. +### Insert below / above (`i` / `I`) + +Inserts a new empty branch into the stack at the cursor position. Lowercase `i` inserts below the cursor (toward trunk); uppercase `I` inserts above the cursor (away from trunk). An inline prompt appears to enter the new branch name. The branch is created at apply time, pointing at the parent branch's tip. + ### Rename (`r`) Opens an inline prompt to enter a new name for the branch. The branch is renamed locally and in the stack metadata. On the next `submit`, the new branch name is pushed to GitHub. ### Reorder (`Shift+↑`/`Shift+↓`) -Moves the selected branch up (away from trunk) or down (toward trunk) in the stack. A cascading rebase adjusts all affected branches. Note: reordering and structural changes (drop/fold/rename) cannot be mixed in the same session. +Moves the selected branch up (away from trunk) or down (toward trunk) in the stack. A cascading rebase adjusts all affected branches. Note: reordering and structural changes (drop/fold/insert/rename) cannot be mixed in the same session. ### Undo (`z`) @@ -60,7 +65,7 @@ Reverses the most recent staged action. You can undo multiple times to step back ## Applying changes -Press `Ctrl+S` to apply all staged changes. Nothing is modified until you save. The apply phase renames branches, folds/drops branches, and runs a cascading rebase to create a linear commit history with the desired stack state. +Press `Ctrl+S` to apply all staged changes. Nothing is modified until you save. The apply phase renames branches, inserts new branches, folds/drops branches, and runs a cascading rebase to create a linear commit history with the desired stack state. ### Handling conflicts @@ -94,8 +99,7 @@ This also works if `modify` was interrupted (e.g., terminal crash). A pre-modify ## Limitations - Cannot modify merged branches (they are locked) -- Cannot add new branches (use `gh stack add` instead) - Cannot split a branch into multiple branches - Cannot move branches between different stacks - Requires an interactive terminal -- Reordering and structural changes (drop/fold/rename) cannot be mixed in the same session +- Reordering and structural changes (drop/fold/insert/rename) cannot be mixed in the same session diff --git a/docs/src/content/docs/guides/ui.md b/docs/src/content/docs/guides/ui.md index 298ef23..0f4f9e4 100644 --- a/docs/src/content/docs/guides/ui.md +++ b/docs/src/content/docs/guides/ui.md @@ -93,7 +93,7 @@ Commits created by a server-side rebase are **not signed**. If your repository r ## Unstacking -If you want to reorder or reorganize the PRs in a stack, you must first dissolve the stack and then re-create it. You can unstack PRs from the UI. +If you want to reorder or reorganize the PRs in a stack from the UI, you must first dissolve the stack and then re-create it. For CLI users, `gh stack modify` provides an interactive way to [restructure a stack](/gh-stack/guides/modify/) — including reordering, inserting, dropping, and renaming branches — without needing to dissolve it. ### Dissolving the Entire Stack diff --git a/docs/src/content/docs/guides/workflows.md b/docs/src/content/docs/guides/workflows.md index 601f4a9..a1285b1 100644 --- a/docs/src/content/docs/guides/workflows.md +++ b/docs/src/content/docs/guides/workflows.md @@ -299,7 +299,7 @@ All branches in a stack should be part of the same feature or project. If you ne ## Restructuring a Stack -When you need to change the composition of a stack — remove a branch, combine branches, change the order, or rename a branch — use `gh stack modify`: +When you need to change the composition of a stack — remove a branch, combine branches, insert a new branch, change the order, or rename a branch — use `gh stack modify`: ```sh # Open the modify TUI @@ -309,6 +309,8 @@ gh stack modify # x → drop a branch # d → fold down (into branch below) # u → fold up (into branch above) +# i → insert below +# I → insert above # Shift+↑/↓ → reorder # r → rename # z → undo diff --git a/docs/src/content/docs/introduction/overview.md b/docs/src/content/docs/introduction/overview.md index 0222f4b..7ff1545 100644 --- a/docs/src/content/docs/introduction/overview.md +++ b/docs/src/content/docs/introduction/overview.md @@ -89,7 +89,8 @@ While the PR UI provides the review and merge experience, the `gh stack` CLI han - **Creating PRs** — `gh stack submit` pushes branches and creates or updates PRs, linking them as a Stack on GitHub. - **Navigating the stack** — `gh stack up`, `down`, `top`, and `bottom` let you move between layers without remembering branch names. - **Syncing everything** — `gh stack sync` fetches, rebases, pushes, and updates PR state in one command. -- **Tearing down stacks** — `gh stack unstack` removes a stack from GitHub and local tracking if you need to restructure it. +- **Restructuring stacks** — `gh stack modify` opens an interactive terminal UI to drop, fold, insert, rename, and reorder branches in a stack. +- **Tearing down stacks** — `gh stack unstack` removes a stack from GitHub and local tracking. - **Checking out a stack** — `gh stack checkout ` pulls down a stack, with all its branches, from GitHub to your local machine. The CLI is not required to use Stacked PRs — the underlying git operations are standard. But it makes the workflow simpler, and you can create Stacked PRs from the CLI instead of the UI. diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index 13a4569..ca0a143 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -192,6 +192,8 @@ The command checks these conditions before opening the TUI: | Drop | `x` | Remove branch and its commits from stack. Local branch and associated PR are preserved. | | Fold down | `d` | Absorb commits into branch below (toward trunk). Folded branch removed from stack. | | Fold up | `u` | Absorb commits into branch above (away from trunk). Folded branch removed from stack. | +| Insert below | `i` | Insert a new empty branch below the cursor (toward trunk). | +| Insert above | `I` | Insert a new empty branch above the cursor (away from trunk). | | Move down | `Shift+↓` | Reorder branch down (toward trunk) in the stack | | Move up | `Shift+↑` | Reorder branch up (away from trunk) in the stack | | Rename | `r` | Rename the branch (opens inline prompt) | @@ -199,7 +201,7 @@ The command checks these conditions before opening the TUI: **Apply phase:** -When you press `Ctrl+S`, the staged changes are applied by renaming branches, folding/dropping branches, and running a cascading rebase to create a linear commit history with the desired stack state. +When you press `Ctrl+S`, the staged changes are applied by renaming branches, inserting new branches, folding/dropping branches, and running a cascading rebase to create a linear commit history with the desired stack state. If a rebase conflict occurs, you can: - Resolve conflicts, stage files, and run `gh stack modify --continue` @@ -234,7 +236,7 @@ You must have a branch from the stack checked out locally. The command targets t Deletes the stack on GitHub first, if it exists, then removes it from local tracking. If the remote deletion fails, the local state is left untouched so you can retry. Use `--local` to skip the remote deletion and only remove local tracking. -This is useful when you need to restructure a stack — remove a branch, reorder branches, rename branches, or make other large changes. After unstacking, use `gh stack init` to re-create the stack with the desired structure — existing branches are adopted automatically. +This is useful when you need to restructure a stack — remove a branch, insert a branch, reorder branches, rename branches, or make other large changes. After unstacking, use `gh stack init` to re-create the stack with the desired structure — existing branches are adopted automatically. | Flag | Description | |------|-------------| diff --git a/internal/modify/apply.go b/internal/modify/apply.go index a95e292..5acd31b 100644 --- a/internal/modify/apply.go +++ b/internal/modify/apply.go @@ -52,9 +52,18 @@ func BuildSnapshot(s *stack.Stack) (Snapshot, error) { func BuildPlan(nodes []modifyview.ModifyBranchNode) []Action { var plan []Action + // When computing move detection, skip inserted nodes since they + // shift the indices of existing nodes. + effectiveIdx := 0 for i, n := range nodes { - if n.PendingAction == nil && n.OriginalPosition == i && !n.Removed { - continue + if n.IsInserted { + // Inserted nodes always have a PendingAction — handle below + } else { + if n.PendingAction == nil && n.OriginalPosition == effectiveIdx && !n.Removed { + effectiveIdx++ + continue + } + effectiveIdx++ } if n.Removed { @@ -69,10 +78,14 @@ func BuildPlan(nodes []modifyview.ModifyBranchNode) []Action { if n.PendingAction.Type == modifyview.ActionRename { action.NewName = n.PendingAction.NewName } + if n.PendingAction.Type == modifyview.ActionInsertBelow || n.PendingAction.Type == modifyview.ActionInsertAbove { + action.NewName = n.PendingAction.NewName + action.NewPosition = i + } plan = append(plan, action) } - if n.OriginalPosition != i && n.PendingAction == nil { + if !n.IsInserted && n.OriginalPosition != i && n.PendingAction == nil { plan = append(plan, Action{ Type: "move", Branch: n.Ref.Branch, @@ -216,7 +229,103 @@ func ApplyPlan( } } - // Step 2: Folds — absorb one branch's commits into an adjacent branch. + // Step 2: Inserts — create new branches and add to stack metadata. + // Process in order so positions are stable. The node's position in the + // non-removed list determines the parent branch. + for _, n := range nodes { + if n.PendingAction == nil { + continue + } + if n.PendingAction.Type != modifyview.ActionInsertBelow && n.PendingAction.Type != modifyview.ActionInsertAbove { + continue + } + + newName := n.PendingAction.NewName + + // Determine the parent branch: find the position of this node among + // the non-removed, non-merged nodes in the apply-order list, then + // look at the branch just before it (toward trunk). + var parentBranch string + insertPos := -1 + + // Determine where in s.Branches the new branch should go. + // Walk the non-removed nodes to find the relative position. + nonRemovedPos := 0 + for _, other := range nodes { + if other.Removed || other.Ref.IsMerged() { + continue + } + if other.Ref.Branch == newName { + insertPos = nonRemovedPos + break + } + nonRemovedPos++ + } + + if insertPos <= 0 { + parentBranch = s.Trunk.Branch + } else { + // Find the branch at insertPos-1 among active branches + activeCount := 0 + for _, b := range s.Branches { + if b.IsMerged() { + continue + } + if activeCount == insertPos-1 { + parentBranch = b.Branch + break + } + activeCount++ + } + if parentBranch == "" { + parentBranch = s.Trunk.Branch + } + } + + // Create the git branch at the parent's tip + if err := git.CreateBranch(newName, parentBranch); err != nil { + unwindErr := Unwind(cfg, gitDir, snapshot, stackIndex, sf, plan) + if unwindErr != nil { + return nil, nil, fmt.Errorf("creating branch %s failed (%v) and unwind failed (%v)", newName, err, unwindErr) + } + return nil, nil, fmt.Errorf("creating branch %s from %s: %w", newName, parentBranch, err) + } + + // Insert BranchRef into s.Branches at the correct position + newRef := stack.BranchRef{Branch: newName} + targetIdx := len(s.Branches) // default: append at end + if insertPos >= 0 { + // Map the active position back to s.Branches index + activeCount := 0 + for j, b := range s.Branches { + if b.IsMerged() { + continue + } + if activeCount == insertPos { + targetIdx = j + break + } + activeCount++ + } + } + s.Branches = append(s.Branches, stack.BranchRef{}) + copy(s.Branches[targetIdx+1:], s.Branches[targetIdx:]) + s.Branches[targetIdx] = newRef + + // Check if the branch above the insertion point has a PR — + // its base changes, so we need a submit + if targetIdx < len(s.Branches)-1 { + above := s.Branches[targetIdx+1] + if above.PullRequest != nil { + affectsPRs = true + } + } + + result.InsertedBranches = append(result.InsertedBranches, newName) + cfg.Successf("Inserted %s after %s", newName, parentBranch) + } + + // Step 3: Folds — absorb one branch's commits into an adjacent branch. // // Fold-down: cherry-pick the folded branch's commits onto the target below. // The target is below in the stack (closer to trunk), so it doesn't @@ -347,7 +456,7 @@ func ApplyPlan( } } - // Step 3: Drops — remove from stack metadata + // Step 4: Drops — remove from stack metadata // Process in reverse order to preserve indices for i := len(nodes) - 1; i >= 0; i-- { n := nodes[i] @@ -373,7 +482,7 @@ func ApplyPlan( cfg.Successf("Dropped %s from stack", dropBranch) } - // Step 4: Reorder — build the desired branch order from the remaining nodes + // Step 5: Reorder — build the desired branch order from the remaining nodes desiredOrder := make([]string, 0) for _, n := range nodes { if n.Removed { @@ -439,7 +548,7 @@ func ApplyPlan( s.Branches = newBranches } - // Step 5: Cascading rebase — rebase each active branch onto its new parent. + // Step 6: Cascading rebase — rebase each active branch onto its new parent. // Use the original parent tip SHA as the oldBase for --onto, so that only // the branch's own commits are replayed onto the new parent. for i, b := range s.Branches { @@ -896,9 +1005,9 @@ func Unwind(cfg *config.Config, gitDir string, snapshot Snapshot, stackIndex int } } - // Clean up branches created by renames during the partial apply + // Clean up branches created by renames or inserts during the partial apply for _, action := range plan { - if action.Type == "rename" && action.NewName != "" { + if action.NewName != "" && (action.Type == "rename" || action.Type == "insert_below" || action.Type == "insert_above") { if !snapshotNames[action.NewName] && git.BranchExists(action.NewName) { _ = git.DeleteBranch(action.NewName, true) } diff --git a/internal/modify/apply_test.go b/internal/modify/apply_test.go index 9921424..4f654c3 100644 --- a/internal/modify/apply_test.go +++ b/internal/modify/apply_test.go @@ -1794,3 +1794,233 @@ func TestApplyPlan_Rename_ChecksOutNewName(t *testing.T) { // Should check out new-A, not A assert.Equal(t, "new-A", lastCheckout) } + +// ─── BuildPlan: Insert ────────────────────────────────────────────────────── + +func TestBuildPlan_Insert(t *testing.T) { + t.Run("insert below produces insert_below action", func(t *testing.T) { + nodes := []modifyview.ModifyBranchNode{ + { + BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "A"}}, + OriginalPosition: 0, + }, + { + BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "new-branch"}}, + OriginalPosition: -1, + IsInserted: true, + PendingAction: &modifyview.PendingAction{Type: modifyview.ActionInsertBelow, NewName: "new-branch"}, + }, + { + BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "B"}}, + OriginalPosition: 1, + }, + } + plan := BuildPlan(nodes) + require.Len(t, plan, 1) + assert.Equal(t, "insert_below", plan[0].Type) + assert.Equal(t, "new-branch", plan[0].Branch) + assert.Equal(t, "new-branch", plan[0].NewName) + assert.Equal(t, 1, plan[0].NewPosition) + }) + + t.Run("insert above produces insert_above action", func(t *testing.T) { + nodes := []modifyview.ModifyBranchNode{ + { + BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "new-branch"}}, + OriginalPosition: -1, + IsInserted: true, + PendingAction: &modifyview.PendingAction{Type: modifyview.ActionInsertAbove, NewName: "new-branch"}, + }, + { + BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "A"}}, + OriginalPosition: 0, + }, + } + plan := BuildPlan(nodes) + require.Len(t, plan, 1) + assert.Equal(t, "insert_above", plan[0].Type) + assert.Equal(t, "new-branch", plan[0].NewName) + assert.Equal(t, 0, plan[0].NewPosition) + }) +} + +// ─── ApplyPlan: Insert ────────────────────────────────────────────────────── + +func TestApplyPlan_Insert(t *testing.T) { + s := stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "A"}, + {Branch: "B"}, + }, + } + + gitDir := t.TempDir() + sf := writeTestStackFile(t, gitDir, s) + + branchSHAs := map[string]string{ + "main": "sha-main", + "A": "sha-A", + "B": "sha-B", + } + + var createCalls []struct{ name, base string } + mock := newApplyMock(gitDir, branchSHAs) + mock.CreateBranchFn = func(name, base string) error { + createCalls = append(createCalls, struct{ name, base string }{name, base}) + return nil + } + + restore := git.SetOps(mock) + defer restore() + + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + + // Insert "new-branch" between A and B (at position 1 in stack order) + nodes := makeNodes(&sf.Stacks[0]) + insertNode := modifyview.ModifyBranchNode{ + BranchNode: stackview.BranchNode{ + Ref: stack.BranchRef{Branch: "new-branch"}, + IsLinear: true, + }, + PendingAction: &modifyview.PendingAction{Type: modifyview.ActionInsertBelow, NewName: "new-branch"}, + OriginalPosition: -1, + IsInserted: true, + } + // Insert between A(0) and B(1) + allNodes := []modifyview.ModifyBranchNode{nodes[0], insertNode, nodes[1]} + + result, conflict, err := ApplyPlan(cfg, gitDir, &sf.Stacks[0], sf, allNodes, "A", noopUpdateBaseSHAs) + require.NoError(t, err) + assert.Nil(t, conflict) + require.NotNil(t, result) + + // Branch should have been created + require.Len(t, createCalls, 1) + assert.Equal(t, "new-branch", createCalls[0].name) + assert.Equal(t, "A", createCalls[0].base) + + // Stack should now have 3 branches: A, new-branch, B + require.Len(t, sf.Stacks[0].Branches, 3) + assert.Equal(t, "A", sf.Stacks[0].Branches[0].Branch) + assert.Equal(t, "new-branch", sf.Stacks[0].Branches[1].Branch) + assert.Equal(t, "B", sf.Stacks[0].Branches[2].Branch) + + // new-branch should be in InsertedBranches + require.Len(t, result.InsertedBranches, 1) + assert.Equal(t, "new-branch", result.InsertedBranches[0]) +} + +func TestApplyPlan_InsertAtStart(t *testing.T) { + s := stack.Stack{ + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "A"}, + {Branch: "B"}, + }, + } + + gitDir := t.TempDir() + sf := writeTestStackFile(t, gitDir, s) + + branchSHAs := map[string]string{ + "main": "sha-main", + "A": "sha-A", + "B": "sha-B", + } + + var createCalls []struct{ name, base string } + mock := newApplyMock(gitDir, branchSHAs) + mock.CreateBranchFn = func(name, base string) error { + createCalls = append(createCalls, struct{ name, base string }{name, base}) + return nil + } + + restore := git.SetOps(mock) + defer restore() + + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + + // Insert "new-branch" at the start (before A) + nodes := makeNodes(&sf.Stacks[0]) + insertNode := modifyview.ModifyBranchNode{ + BranchNode: stackview.BranchNode{ + Ref: stack.BranchRef{Branch: "new-branch"}, + IsLinear: true, + }, + PendingAction: &modifyview.PendingAction{Type: modifyview.ActionInsertAbove, NewName: "new-branch"}, + OriginalPosition: -1, + IsInserted: true, + } + allNodes := []modifyview.ModifyBranchNode{insertNode, nodes[0], nodes[1]} + + result, conflict, err := ApplyPlan(cfg, gitDir, &sf.Stacks[0], sf, allNodes, "A", noopUpdateBaseSHAs) + require.NoError(t, err) + assert.Nil(t, conflict) + require.NotNil(t, result) + + // Branch should be created from trunk + require.Len(t, createCalls, 1) + assert.Equal(t, "new-branch", createCalls[0].name) + assert.Equal(t, "main", createCalls[0].base) + + // Stack should now have 3 branches: new-branch, A, B + require.Len(t, sf.Stacks[0].Branches, 3) + assert.Equal(t, "new-branch", sf.Stacks[0].Branches[0].Branch) + assert.Equal(t, "A", sf.Stacks[0].Branches[1].Branch) + assert.Equal(t, "B", sf.Stacks[0].Branches[2].Branch) +} + +func TestApplyPlan_InsertAffectsPRs(t *testing.T) { + s := stack.Stack{ + ID: "test-id", + Trunk: stack.BranchRef{Branch: "main"}, + Branches: []stack.BranchRef{ + {Branch: "A"}, + {Branch: "B", PullRequest: &stack.PullRequestRef{Number: 42}}, + }, + } + + gitDir := t.TempDir() + sf := writeTestStackFile(t, gitDir, s) + + branchSHAs := map[string]string{ + "main": "sha-main", + "A": "sha-A", + "B": "sha-B", + } + + mock := newApplyMock(gitDir, branchSHAs) + mock.CreateBranchFn = func(name, base string) error { return nil } + + restore := git.SetOps(mock) + defer restore() + + cfg, _, _ := config.NewTestConfig() + defer cfg.Out.Close() + defer cfg.Err.Close() + + // Insert between A and B — B has a PR, so base changes → affectsPRs + nodes := makeNodes(&sf.Stacks[0]) + insertNode := modifyview.ModifyBranchNode{ + BranchNode: stackview.BranchNode{ + Ref: stack.BranchRef{Branch: "new-branch"}, + IsLinear: true, + }, + PendingAction: &modifyview.PendingAction{Type: modifyview.ActionInsertBelow, NewName: "new-branch"}, + OriginalPosition: -1, + IsInserted: true, + } + allNodes := []modifyview.ModifyBranchNode{nodes[0], insertNode, nodes[1]} + + result, _, err := ApplyPlan(cfg, gitDir, &sf.Stacks[0], sf, allNodes, "A", noopUpdateBaseSHAs) + require.NoError(t, err) + require.NotNil(t, result) + + // Should need submit because insertion changes the base of a branch with PR + assert.True(t, result.NeedsSubmit, "inserting before a branch with a PR should trigger NeedsSubmit") +} diff --git a/internal/tui/modifyview/help.go b/internal/tui/modifyview/help.go index ad25db3..f43de88 100644 --- a/internal/tui/modifyview/help.go +++ b/internal/tui/modifyview/help.go @@ -13,7 +13,7 @@ func renderHelpOverlay(width, height int) string { title := helpTitleStyle.Render("Modify Stack") b.WriteString(title) b.WriteString("\n") - b.WriteString(helpDescStyle.Render("Restructure your stack by dropping, folding, renaming, or reordering branches.")) + b.WriteString(helpDescStyle.Render("Restructure your stack by dropping, folding, inserting, renaming, or reordering branches.")) b.WriteString("\n") sections := []struct { @@ -25,8 +25,12 @@ func renderHelpOverlay(width, height int) string { "Remove a branch and its commits from the stack.\nThe local branch is preserved; the PR stays open on GitHub.", }, { - "Fold up / down (u / d)", - "Merge a branch's commits into an adjacent branch.\nFold up absorbs into the branch above; fold down into the branch below.", + "Fold down / up (d / u)", + "Merge a branch's commits into an adjacent branch.\nFold down absorbs into the branch below; fold up into the branch above.", + }, + { + "Insert below / above (i / I)", + "Insert a new empty branch into the stack.\nLowercase i inserts below the cursor; uppercase I inserts above.", }, { "Rename (r)", diff --git a/internal/tui/modifyview/model.go b/internal/tui/modifyview/model.go index eff9e79..169e81b 100644 --- a/internal/tui/modifyview/model.go +++ b/internal/tui/modifyview/model.go @@ -11,6 +11,7 @@ import ( "github.com/github/gh-stack/internal/git" "github.com/github/gh-stack/internal/stack" "github.com/github/gh-stack/internal/tui/shared" + "github.com/github/gh-stack/internal/tui/stackview" ) // modifyKeyMap defines key bindings for the modify view. @@ -23,6 +24,8 @@ type modifyKeyMap struct { FoldDown key.Binding FoldUp key.Binding Rename key.Binding + InsertBelow key.Binding + InsertAbove key.Binding Undo key.Binding ToggleCommits key.Binding ToggleFiles key.Binding @@ -32,7 +35,7 @@ type modifyKeyMap struct { } func (k modifyKeyMap) ShortHelp() []key.Binding { - return []key.Binding{k.Up, k.Down, k.Drop, k.FoldDown, k.Rename, k.ToggleCommits, k.ToggleFiles, k.Apply, k.Help, k.Quit} + return []key.Binding{k.Up, k.Down, k.Drop, k.FoldDown, k.InsertBelow, k.Rename, k.ToggleCommits, k.ToggleFiles, k.Apply, k.Help, k.Quit} } func (k modifyKeyMap) FullHelp() [][]key.Binding { @@ -72,6 +75,14 @@ var modifyKeys = modifyKeyMap{ key.WithKeys("r"), key.WithHelp("r", "rename"), ), + InsertBelow: key.NewBinding( + key.WithKeys("i"), + key.WithHelp("i", "insert below"), + ), + InsertAbove: key.NewBinding( + key.WithKeys("I"), + key.WithHelp("I", "insert above"), + ), Undo: key.NewBinding( key.WithKeys("z"), key.WithHelp("z", "undo"), @@ -116,6 +127,11 @@ type Model struct { renameInput textinput.Model renameOriginal string // original branch name shown as label + // Insert mode + insertMode bool + insertDirection ActionType // ActionInsertBelow or ActionInsertAbove + insertInput textinput.Model + // Help overlay showHelp bool @@ -139,6 +155,9 @@ func New(nodes []ModifyBranchNode, trunk stack.BranchRef, version string) Model ti := textinput.New() ti.CharLimit = 100 + ii := textinput.New() + ii.CharLimit = 100 + // Default cursor to the current active branch, or first non-merged branch cursor := 0 found := false @@ -164,6 +183,7 @@ func New(nodes []ModifyBranchNode, trunk stack.BranchRef, version string) Model version: version, cursor: cursor, renameInput: ti, + insertInput: ii, } } @@ -210,6 +230,9 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if m.renameMode { return m.updateRename(msg) } + if m.insertMode { + return m.updateInsert(msg) + } return m.updateNormal(msg) case tea.MouseMsg: @@ -290,6 +313,24 @@ func (m Model) updateRename(msg tea.KeyMsg) (tea.Model, tea.Cmd) { } } + // For inserted nodes, update the insert action's name directly + // rather than creating a separate rename action. + if node.IsInserted { + oldInsertName := node.Ref.Branch + node.Ref.Branch = newName + node.PendingAction.NewName = newName + // Update the matching undo stack entry + for i := len(m.actionStack) - 1; i >= 0; i-- { + a := &m.actionStack[i] + if (a.Type == ActionInsertBelow || a.Type == ActionInsertAbove) && a.BranchName == oldInsertName { + a.BranchName = newName + break + } + } + m.renameMode = false + return m, nil + } + // Record undo action m.actionStack = append(m.actionStack, StagedAction{ Type: ActionRename, @@ -316,7 +357,94 @@ func (m Model) updateRename(msg tea.KeyMsg) (tea.Model, tea.Cmd) { } } -// updateNormal handles keys during normal (non-modal) interaction. +// updateInsert handles keys while in insert mode (typing a new branch name). +func (m Model) updateInsert(msg tea.KeyMsg) (tea.Model, tea.Cmd) { + switch msg.Type { + case tea.KeyEnter: + newName := strings.TrimSpace(m.insertInput.Value()) + if newName == "" { + m.insertMode = false + return m, nil + } + + // Validate: git ref name rules + if err := git.ValidateRefName(newName); err != nil { + m.statusMessage = fmt.Sprintf("Invalid branch name: %s", err) + m.statusIsError = true + return m, nil + } + + // Validate: not already used by another local branch + if git.BranchExists(newName) { + m.statusMessage = fmt.Sprintf("Branch %q already exists locally", newName) + m.statusIsError = true + return m, nil + } + + // Validate: not already used in this stack (by another node) + for _, other := range m.nodes { + checkName := other.Ref.Branch + if other.PendingAction != nil && other.PendingAction.Type == ActionRename { + checkName = other.PendingAction.NewName + } + if other.PendingAction != nil && (other.PendingAction.Type == ActionInsertBelow || other.PendingAction.Type == ActionInsertAbove) { + checkName = other.PendingAction.NewName + } + if checkName == newName { + m.statusMessage = fmt.Sprintf("Branch %q already used in this stack", newName) + m.statusIsError = true + return m, nil + } + } + + // Determine insertion position + insertIdx := m.cursor + if m.insertDirection == ActionInsertBelow { + insertIdx = m.cursor + 1 + } + // For InsertAbove, insertIdx stays at m.cursor (insert before cursor) + + // Create the new node + newNode := ModifyBranchNode{ + BranchNode: stackview.BranchNode{ + Ref: stack.BranchRef{Branch: newName}, + IsLinear: true, + }, + PendingAction: &PendingAction{ + Type: m.insertDirection, + NewName: newName, + }, + OriginalPosition: -1, // sentinel: this node has no original position + IsInserted: true, + } + + // Insert the node into the slice + m.nodes = append(m.nodes, ModifyBranchNode{}) + copy(m.nodes[insertIdx+1:], m.nodes[insertIdx:]) + m.nodes[insertIdx] = newNode + + // Record undo action + m.actionStack = append(m.actionStack, StagedAction{ + Type: m.insertDirection, + BranchName: newName, + }) + + // Move cursor to the newly inserted node + m.cursor = insertIdx + m.insertMode = false + m.ensureVisible() + return m, nil + + case tea.KeyEscape: + m.insertMode = false + return m, nil + + default: + var cmd tea.Cmd + m.insertInput, cmd = m.insertInput.Update(msg) + return m, cmd + } +} func (m Model) updateNormal(msg tea.KeyMsg) (tea.Model, tea.Cmd) { switch { case key.Matches(msg, modifyKeys.Quit): @@ -333,7 +461,7 @@ func (m Model) updateNormal(msg tea.KeyMsg) (tea.Model, tea.Cmd) { case key.Matches(msg, modifyKeys.MoveUp): if m.currentMode() == modeStructure { - m.statusMessage = "Cannot reorder while drops, folds, or renames are staged — undo them first" + m.statusMessage = "Cannot reorder while drops, folds, inserts, or renames are staged — undo them first" m.statusIsError = true return m, nil } @@ -342,7 +470,7 @@ func (m Model) updateNormal(msg tea.KeyMsg) (tea.Model, tea.Cmd) { case key.Matches(msg, modifyKeys.MoveDown): if m.currentMode() == modeStructure { - m.statusMessage = "Cannot reorder while drops, folds, or renames are staged — undo them first" + m.statusMessage = "Cannot reorder while drops, folds, inserts, or renames are staged — undo them first" m.statusIsError = true return m, nil } @@ -385,6 +513,24 @@ func (m Model) updateNormal(msg tea.KeyMsg) (tea.Model, tea.Cmd) { m.startRename() return m, nil + case key.Matches(msg, modifyKeys.InsertBelow): + if m.currentMode() == modeReorder { + m.statusMessage = "Cannot insert while branches are reordered — undo moves first" + m.statusIsError = true + return m, nil + } + m.startInsert(ActionInsertBelow) + return m, nil + + case key.Matches(msg, modifyKeys.InsertAbove): + if m.currentMode() == modeReorder { + m.statusMessage = "Cannot insert while branches are reordered — undo moves first" + m.statusIsError = true + return m, nil + } + m.startInsert(ActionInsertAbove) + return m, nil + case key.Matches(msg, modifyKeys.Undo): m.undoLast() return m, nil @@ -432,17 +578,28 @@ func (m *Model) currentMode() actionMode { hasReorder := false hasStructure := false - for i, n := range m.nodes { + for _, n := range m.nodes { if n.PendingAction != nil { switch n.PendingAction.Type { - case ActionDrop, ActionFoldDown, ActionFoldUp, ActionRename: + case ActionDrop, ActionFoldDown, ActionFoldUp, ActionRename, ActionInsertBelow, ActionInsertAbove: hasStructure = true } } - // Position change without explicit action = reorder - if !n.Ref.IsMerged() && n.OriginalPosition != i && n.PendingAction == nil { + } + + // Position change without explicit action = reorder. + // Skip inserted nodes — they don't have an original position and + // their presence shifts indices of other nodes. + effectiveIdx := 0 + for _, n := range m.nodes { + if n.IsInserted { + continue + } + if !n.Ref.IsMerged() && n.OriginalPosition != effectiveIdx && n.PendingAction == nil { hasReorder = true + break } + effectiveIdx++ } if hasReorder { @@ -525,6 +682,30 @@ func (m *Model) toggleDrop() { return } + // Dropping an inserted node removes it entirely (undo the insert). + // Pop the original insert action from the undo stack rather than + // pushing a new entry — this makes the drop behave as a direct + // cancellation of the insert. + if node.IsInserted { + branchName := node.Ref.Branch + m.nodes = append(m.nodes[:m.cursor], m.nodes[m.cursor+1:]...) + // Remove the matching insert action from the undo stack + for i := len(m.actionStack) - 1; i >= 0; i-- { + a := m.actionStack[i] + if (a.Type == ActionInsertBelow || a.Type == ActionInsertAbove) && a.BranchName == branchName { + m.actionStack = append(m.actionStack[:i], m.actionStack[i+1:]...) + break + } + } + if m.cursor >= len(m.nodes) { + m.cursor = len(m.nodes) - 1 + } + if m.cursor < 0 { + m.cursor = 0 + } + return + } + if node.PendingAction != nil && node.PendingAction.Type == ActionDrop { // Undo drop m.actionStack = append(m.actionStack, StagedAction{ @@ -537,6 +718,8 @@ func (m *Model) toggleDrop() { // Check if any other branch has a fold targeting this branch. // A fold-up targets the branch above (lower index), fold-down // targets the branch below (higher index). + // Also check if dropping this branch would cause a fold to + // retarget to an inserted branch. for i, other := range m.nodes { if other.PendingAction == nil || i == m.cursor { continue @@ -546,6 +729,25 @@ func (m *Model) toggleDrop() { for j := i - 1; j >= 0; j-- { if !m.nodes[j].Removed && !m.nodes[j].Ref.IsMerged() { if j == m.cursor { + // This branch is the current target. Check what the + // next target would be after dropping it. + nextTarget := -1 + for k := j - 1; k >= 0; k-- { + if !m.nodes[k].Removed && !m.nodes[k].Ref.IsMerged() { + nextTarget = k + break + } + } + if nextTarget < 0 { + m.statusMessage = fmt.Sprintf("Cannot drop: %s is folding into this branch", other.Ref.Branch) + m.statusIsError = true + return + } + if m.nodes[nextTarget].IsInserted { + m.statusMessage = fmt.Sprintf("Cannot drop: %s would fold into an inserted branch", other.Ref.Branch) + m.statusIsError = true + return + } m.statusMessage = fmt.Sprintf("Cannot drop: %s is folding into this branch", other.Ref.Branch) m.statusIsError = true return @@ -559,6 +761,25 @@ func (m *Model) toggleDrop() { for j := i + 1; j < len(m.nodes); j++ { if !m.nodes[j].Removed && !m.nodes[j].Ref.IsMerged() { if j == m.cursor { + // This branch is the current target. Check what the + // next target would be after dropping it. + nextTarget := -1 + for k := j + 1; k < len(m.nodes); k++ { + if !m.nodes[k].Removed && !m.nodes[k].Ref.IsMerged() { + nextTarget = k + break + } + } + if nextTarget < 0 { + m.statusMessage = fmt.Sprintf("Cannot drop: %s is folding into this branch", other.Ref.Branch) + m.statusIsError = true + return + } + if m.nodes[nextTarget].IsInserted { + m.statusMessage = fmt.Sprintf("Cannot drop: %s would fold into an inserted branch", other.Ref.Branch) + m.statusIsError = true + return + } m.statusMessage = fmt.Sprintf("Cannot drop: %s is folding into this branch", other.Ref.Branch) m.statusIsError = true return @@ -569,13 +790,13 @@ func (m *Model) toggleDrop() { } } - // Check if this would remove the last active branch + // Check if this would remove the last active original branch active := 0 for j, other := range m.nodes { if j == m.cursor { continue // skip the branch we're about to drop } - if !other.Removed && !other.Ref.IsMerged() { + if !other.Removed && !other.Ref.IsMerged() && !other.IsInserted { active++ } } @@ -601,6 +822,11 @@ func (m *Model) fold(action ActionType) { return } node := &m.nodes[m.cursor] + if node.IsInserted { + m.statusMessage = "Cannot fold an inserted branch — drop it with x to remove" + m.statusIsError = true + return + } if node.Ref.IsMerged() { m.statusMessage = "Cannot fold a merged branch" m.statusIsError = true @@ -630,6 +856,38 @@ func (m *Model) fold(action ActionType) { return } + // Check if the current node is the target of another fold — folding + // a branch that is receiving commits from another fold is not allowed. + for i, other := range m.nodes { + if other.PendingAction == nil || i == m.cursor { + continue + } + if other.PendingAction.Type == ActionFoldUp { + for j := i - 1; j >= 0; j-- { + if !m.nodes[j].Removed && !m.nodes[j].Ref.IsMerged() { + if j == m.cursor { + m.statusMessage = fmt.Sprintf("Cannot fold: %s is folding into this branch", other.Ref.Branch) + m.statusIsError = true + return + } + break + } + } + } + if other.PendingAction.Type == ActionFoldDown { + for j := i + 1; j < len(m.nodes); j++ { + if !m.nodes[j].Removed && !m.nodes[j].Ref.IsMerged() { + if j == m.cursor { + m.statusMessage = fmt.Sprintf("Cannot fold: %s is folding into this branch", other.Ref.Branch) + m.statusIsError = true + return + } + break + } + } + } + } + // Find the target branch var targetIdx int var found bool @@ -648,6 +906,11 @@ func (m *Model) fold(action ActionType) { m.statusIsError = true return } + if m.nodes[targetIdx].IsInserted { + m.statusMessage = "Cannot fold into an inserted branch" + m.statusIsError = true + return + } } else { // Fold up: target is the previous non-removed, non-merged node away from trunk (lower index) for i := m.cursor - 1; i >= 0; i-- { @@ -662,15 +925,28 @@ func (m *Model) fold(action ActionType) { m.statusIsError = true return } + if m.nodes[targetIdx].IsInserted { + m.statusMessage = "Cannot fold into an inserted branch" + m.statusIsError = true + return + } + } + + // Check if the target is already folding (mutual fold / chain fold) + target := &m.nodes[targetIdx] + if target.PendingAction != nil && (target.PendingAction.Type == ActionFoldDown || target.PendingAction.Type == ActionFoldUp) { + m.statusMessage = fmt.Sprintf("Cannot fold: %s is already folding in the opposite direction", target.Ref.Branch) + m.statusIsError = true + return } - // Check if this would remove the last active branch + // Check if this would remove the last active original branch active := 0 for j, other := range m.nodes { if j == m.cursor { continue } - if !other.Removed && !other.Ref.IsMerged() { + if !other.Removed && !other.Ref.IsMerged() && !other.IsInserted { active++ } } @@ -712,6 +988,66 @@ func (m *Model) startRename() { m.renameInput.CursorEnd() } +// startInsert enters insert mode to type a new branch name. +func (m *Model) startInsert(direction ActionType) { + if m.cursor < 0 || m.cursor >= len(m.nodes) { + return + } + node := &m.nodes[m.cursor] + if node.Ref.IsMerged() { + m.statusMessage = "Cannot insert next to a merged branch" + m.statusIsError = true + return + } + if node.Removed { + return + } + + // Compute where the node would be inserted + insertIdx := m.cursor + if direction == ActionInsertBelow { + insertIdx = m.cursor + 1 + } + + // Check if inserting here would place the new branch between a + // folding branch and its target, making it the new fold target. + for i, other := range m.nodes { + if other.PendingAction == nil { + continue + } + if other.PendingAction.Type == ActionFoldDown { + for j := i + 1; j < len(m.nodes); j++ { + if !m.nodes[j].Removed && !m.nodes[j].Ref.IsMerged() { + if insertIdx > i && insertIdx <= j { + m.statusMessage = fmt.Sprintf("Cannot insert here: %s is folding into %s", other.Ref.Branch, m.nodes[j].Ref.Branch) + m.statusIsError = true + return + } + break + } + } + } + if other.PendingAction.Type == ActionFoldUp { + for j := i - 1; j >= 0; j-- { + if !m.nodes[j].Removed && !m.nodes[j].Ref.IsMerged() { + if insertIdx > j && insertIdx <= i { + m.statusMessage = fmt.Sprintf("Cannot insert here: %s is folding into %s", other.Ref.Branch, m.nodes[j].Ref.Branch) + m.statusIsError = true + return + } + break + } + } + } + } + + m.insertMode = true + m.insertDirection = direction + m.insertInput.SetValue("") + m.insertInput.Prompt = "" + m.insertInput.Focus() +} + // undoLast reverses the most recent action from the stack. func (m *Model) undoLast() { if len(m.actionStack) == 0 { @@ -775,21 +1111,41 @@ func (m *Model) undoLast() { break } } + + case ActionInsertBelow, ActionInsertAbove: + // Remove the inserted node from the slice + for i := range m.nodes { + if m.nodes[i].IsInserted && m.nodes[i].Ref.Branch == action.BranchName { + m.nodes = append(m.nodes[:i], m.nodes[i+1:]...) + if m.cursor >= len(m.nodes) { + m.cursor = len(m.nodes) - 1 + } + if m.cursor < 0 { + m.cursor = 0 + } + break + } + } } } // tryApply validates and initiates apply. func (m Model) tryApply() (tea.Model, tea.Cmd) { hasPending := false - for i, n := range m.nodes { + effectiveIdx := 0 + for _, n := range m.nodes { if n.PendingAction != nil { hasPending = true break } - if !n.Removed && n.OriginalPosition != i { + if n.IsInserted { + continue + } + if !n.Removed && n.OriginalPosition != effectiveIdx { hasPending = true break } + effectiveIdx++ } if !hasPending { @@ -798,10 +1154,10 @@ func (m Model) tryApply() (tea.Model, tea.Cmd) { return m, nil } - // Ensure at least one non-removed, non-merged branch remains + // Ensure at least one non-removed, non-merged, non-inserted branch remains active := 0 for _, n := range m.nodes { - if !n.Removed && !n.Ref.IsMerged() { + if !n.Removed && !n.Ref.IsMerged() && !n.IsInserted { active++ } } @@ -832,7 +1188,7 @@ func (m *Model) ensureVisible() { } func (m Model) nodeLineCount(idx int) int { - return shared.NodeLineCount(toNodeData(m.nodes[idx], idx)) + return shared.NodeLineCount(toNodeData(m.nodes[idx], idx, idx)) } func (m Model) contentViewHeight() int { @@ -862,7 +1218,7 @@ func (m *Model) clampScroll() { func (m Model) handleMouseClick(screenX, screenY int) (tea.Model, tea.Cmd) { nodes := make([]shared.BranchNodeData, len(m.nodes)) for i, n := range m.nodes { - nodes[i] = toNodeData(n, i) + nodes[i] = toNodeData(n, i, i) } result := shared.HandleClick(screenX, screenY, nodes, m.width, m.height, m.scrollOffset, shared.ShouldShowHeader(m.width, m.height), false) @@ -896,8 +1252,9 @@ func (m Model) handleMouseClick(screenX, screenY int) (tea.Model, tea.Cmd) { // toNodeData converts a ModifyBranchNode to shared.BranchNodeData, // applying drop/fold/move visual overrides. currentIdx is the node's -// current position in the list, used to detect moves. -func toNodeData(n ModifyBranchNode, currentIdx int) shared.BranchNodeData { +// current position in the list. effectiveIdx is the position among +// non-inserted nodes (used for move detection). +func toNodeData(n ModifyBranchNode, currentIdx int, effectiveIdx int) shared.BranchNodeData { data := shared.BranchNodeData{ Ref: n.Ref, IsCurrent: n.IsCurrent, @@ -926,11 +1283,16 @@ func toNodeData(n ModifyBranchNode, currentIdx int) shared.BranchNodeData { data.BranchNameStyleOverride = &s data.ConnectorStyleOverride = &c data.ForceDashedConnector = true + case ActionInsertBelow, ActionInsertAbove: + s := insertBranchStyle + c := insertConnectorStyle + data.BranchNameStyleOverride = &s + data.ConnectorStyleOverride = &c } } // Moved branch: purple solid connector (no dash, no strikethrough) - if n.PendingAction == nil && !n.Ref.IsMerged() && n.OriginalPosition != currentIdx { + if n.PendingAction == nil && !n.Ref.IsMerged() && !n.IsInserted && n.OriginalPosition != effectiveIdx { c := movedConnectorStyle data.ConnectorStyleOverride = &c } @@ -939,8 +1301,9 @@ func toNodeData(n ModifyBranchNode, currentIdx int) shared.BranchNodeData { } // nodeAnnotation builds an optional annotation from the node's pending action -// or its position change. currentIdx is the node's current position in the list. -func nodeAnnotation(n ModifyBranchNode, currentIdx int) *shared.NodeAnnotation { +// or its position change. effectiveIdx is the node's position among non-inserted +// nodes, used for move detection. +func nodeAnnotation(n ModifyBranchNode, effectiveIdx int) *shared.NodeAnnotation { if n.Ref.IsMerged() { return &shared.NodeAnnotation{Text: "🔒", Style: shared.DimStyle} } @@ -954,13 +1317,15 @@ func nodeAnnotation(n ModifyBranchNode, currentIdx int) *shared.NodeAnnotation { return &shared.NodeAnnotation{Text: "↑ fold up", Style: foldBadge} case ActionRename: return &shared.NodeAnnotation{Text: "→ " + n.PendingAction.NewName, Style: renameBadge} + case ActionInsertBelow, ActionInsertAbove: + return &shared.NodeAnnotation{Text: "✚ insert", Style: insertBadge} case ActionMove: return &shared.NodeAnnotation{Text: "↕ moved", Style: moveBadge} } } // Show move annotation when position changed (even without explicit PendingAction) - if !n.Ref.IsMerged() && n.OriginalPosition != currentIdx { - delta := n.OriginalPosition - currentIdx // positive = moved up (toward top) + if !n.Ref.IsMerged() && !n.IsInserted && n.OriginalPosition != effectiveIdx { + delta := n.OriginalPosition - effectiveIdx // positive = moved up (toward top) direction := "up" layers := delta if delta < 0 { @@ -996,10 +1361,17 @@ func (m Model) View() string { // Build the scrollable branch list content var b strings.Builder + effectiveIdx := 0 for i := 0; i < len(m.nodes); i++ { - nodeData := toNodeData(m.nodes[i], i) + ei := effectiveIdx + if m.nodes[i].IsInserted { + ei = -1 // inserted nodes have no effective position + } else { + effectiveIdx++ + } + nodeData := toNodeData(m.nodes[i], i, ei) isFocused := i == m.cursor - annotation := nodeAnnotation(m.nodes[i], i) + annotation := nodeAnnotation(m.nodes[i], ei) shared.RenderNode(&b, nodeData, isFocused, m.width, annotation) } shared.RenderTrunk(&b, m.trunk.Branch) @@ -1036,6 +1408,13 @@ func (m Model) View() string { if m.renameMode { out.WriteString(renameBadge.Render(fmt.Sprintf("Rename: %s → ", m.renameOriginal))) out.WriteString(m.renameInput.View()) + } else if m.insertMode { + direction := "below" + if m.insertDirection == ActionInsertAbove { + direction = "above" + } + out.WriteString(insertBadge.Render(fmt.Sprintf("Insert %s: ", direction))) + out.WriteString(m.insertInput.View()) } else { out.WriteString(renderStatusLine(m.nodes, m.width)) } @@ -1052,7 +1431,13 @@ func (m Model) buildHeaderConfig() shared.HeaderConfig { } } - branchCount := len(m.nodes) + // Count only original branches (exclude staged inserts) + branchCount := 0 + for _, n := range m.nodes { + if !n.IsInserted { + branchCount++ + } + } branchInfo := fmt.Sprintf("%d branches", branchCount) if branchCount == 1 { branchInfo = "1 branch" @@ -1088,8 +1473,8 @@ func (m Model) buildHeaderConfig() shared.HeaderConfig { // Left column // Right column {Key: "↑↓", Desc: "select branch"}, {Key: "x", Desc: "drop", Disabled: structureDisabled}, {Key: "f", Desc: "view files"}, {Key: "r", Desc: "rename", Disabled: structureDisabled}, - {Key: "c", Desc: "view commits"}, {Key: "u", Desc: "fold up", Disabled: structureDisabled}, - {Key: "?", Desc: "help"}, {Key: "d", Desc: "fold down", Disabled: structureDisabled}, + {Key: "c", Desc: "view commits"}, {Key: "i/I", Desc: "insert below/above", Disabled: structureDisabled}, + {Key: "?", Desc: "help"}, {Key: "d/u", Desc: "fold down/up", Disabled: structureDisabled}, {Key: "q/esc", Desc: "quit"}, {Key: "shift+↑↓", Desc: "reorder", Disabled: reorderDisabled}, {Key: "^S", Desc: "apply changes"}, {Key: "z", Desc: "undo"}, }, diff --git a/internal/tui/modifyview/model_test.go b/internal/tui/modifyview/model_test.go index 6afc13d..81675ad 100644 --- a/internal/tui/modifyview/model_test.go +++ b/internal/tui/modifyview/model_test.go @@ -205,11 +205,10 @@ func TestCannotFoldLastBranch(t *testing.T) { // --- Mutual fold test --- -func TestMutualFoldBlocked(t *testing.T) { +func TestFoldTargetOfFoldBlocked(t *testing.T) { // With 3 nodes A(0), B(1), C(2): fold B down into C, then try - // to fold C up. Since B is removed the target search for fold-up - // skips B and finds A. The fold into A would leave only 1 active - // branch (A) so it IS allowed (active >= 1). Verify the behavior. + // to fold C up. C is the target of B's fold, so C should not + // be allowed to fold. nodes := []ModifyBranchNode{ makeNode("a", false, 0), makeNode("b", true, 1), @@ -227,21 +226,11 @@ func TestMutualFoldBlocked(t *testing.T) { m = sendKey(t, m, runeKey('j')) require.Equal(t, 2, m.cursor) - // Try fold C up — B is removed so target becomes A. - // With only 2 nodes removed (B, C), A is the only active → active=1 (passes guard). + // Try fold C up — C is the target of B's fold-down, so blocked m = sendKey(t, m, runeKey('u')) - require.NotNil(t, m.nodes[2].PendingAction, "C should fold up into A since B is skipped") - assert.Equal(t, ActionFoldUp, m.nodes[2].PendingAction.Type) - assert.True(t, m.nodes[2].Removed) - - // Verify only A remains active - active := 0 - for _, n := range m.nodes { - if !n.Removed { - active++ - } - } - assert.Equal(t, 1, active, "only A should remain active") + assert.Nil(t, m.nodes[2].PendingAction, "C should not fold because B is folding into it") + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "folding into this branch") } // --- Mode exclusivity tests --- @@ -774,3 +763,794 @@ func TestCursorNavigation_SkipsMergedBranches(t *testing.T) { m = sendKey(t, m, runeKey('k')) assert.Equal(t, 0, m.cursor, "up should skip merged branch") } + +// --- Insert tests --- + +// simulateInsert enters insert mode, types a branch name, and presses Enter. +func simulateInsert(t *testing.T, m Model, name string) Model { + t.Helper() + require.True(t, m.insertMode, "expected insert mode to be active") + for _, r := range name { + m = sendKey(t, m, runeKey(r)) + } + m = sendKey(t, m, tea.KeyMsg{Type: tea.KeyEnter}) + return m +} + +func TestInsertBelow(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + require.Equal(t, 0, m.cursor) // cursor on a + + // Press 'i' to insert below + m = sendKey(t, m, runeKey('i')) + require.True(t, m.insertMode, "should enter insert mode") + assert.Equal(t, ActionInsertBelow, m.insertDirection) + + // Type a branch name and confirm + m = simulateInsert(t, m, "new-branch") + require.False(t, m.insertMode, "should exit insert mode") + + // New node should be at index 1 (below cursor at 0) + require.Len(t, m.nodes, 4) + assert.Equal(t, "a", m.nodes[0].Ref.Branch) + assert.Equal(t, "new-branch", m.nodes[1].Ref.Branch) + assert.True(t, m.nodes[1].IsInserted) + assert.NotNil(t, m.nodes[1].PendingAction) + assert.Equal(t, ActionInsertBelow, m.nodes[1].PendingAction.Type) + assert.Equal(t, "new-branch", m.nodes[1].PendingAction.NewName) + assert.Equal(t, "b", m.nodes[2].Ref.Branch) + assert.Equal(t, "c", m.nodes[3].Ref.Branch) + + // Cursor should be on the new node + assert.Equal(t, 1, m.cursor) +} + +func TestInsertAbove(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", false, 0), + makeNode("b", true, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + require.Equal(t, 1, m.cursor) // cursor on b + + // Press 'I' (shift+i) to insert above + m = sendKey(t, m, runeKey('I')) + require.True(t, m.insertMode, "should enter insert mode") + assert.Equal(t, ActionInsertAbove, m.insertDirection) + + // Type a branch name and confirm + m = simulateInsert(t, m, "above-b") + require.False(t, m.insertMode, "should exit insert mode") + + // New node should be at index 1 (above cursor which was at 1) + require.Len(t, m.nodes, 4) + assert.Equal(t, "a", m.nodes[0].Ref.Branch) + assert.Equal(t, "above-b", m.nodes[1].Ref.Branch) + assert.True(t, m.nodes[1].IsInserted) + assert.NotNil(t, m.nodes[1].PendingAction) + assert.Equal(t, ActionInsertAbove, m.nodes[1].PendingAction.Type) + assert.Equal(t, "b", m.nodes[2].Ref.Branch) + assert.Equal(t, "c", m.nodes[3].Ref.Branch) + + // Cursor should be on the new node + assert.Equal(t, 1, m.cursor) +} + +func TestInsertAtTop(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + } + m := New(nodes, testTrunk, "1.0.0") + require.Equal(t, 0, m.cursor) + + // Insert above the top — new node becomes index 0 + m = sendKey(t, m, runeKey('I')) + m = simulateInsert(t, m, "top") + + require.Len(t, m.nodes, 3) + assert.Equal(t, "top", m.nodes[0].Ref.Branch) + assert.True(t, m.nodes[0].IsInserted) + assert.Equal(t, "a", m.nodes[1].Ref.Branch) + assert.Equal(t, "b", m.nodes[2].Ref.Branch) + assert.Equal(t, 0, m.cursor) +} + +func TestInsertAtBottom(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", false, 0), + makeNode("b", true, 1), + } + m := New(nodes, testTrunk, "1.0.0") + // Move cursor to bottom + m = sendKey(t, m, runeKey('j')) + // cursor already starts at 1 (b is current) + require.Equal(t, 1, m.cursor) + + // Insert below the bottom — new node appended + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "bottom") + + require.Len(t, m.nodes, 3) + assert.Equal(t, "a", m.nodes[0].Ref.Branch) + assert.Equal(t, "b", m.nodes[1].Ref.Branch) + assert.Equal(t, "bottom", m.nodes[2].Ref.Branch) + assert.True(t, m.nodes[2].IsInserted) + assert.Equal(t, 2, m.cursor) +} + +func TestUndoInsert(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Insert below a + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + require.Len(t, m.nodes, 4) + assert.Equal(t, 1, m.cursor) + + // Undo + m = sendKey(t, m, runeKey('z')) + require.Len(t, m.nodes, 3, "inserted node should be removed") + assert.Equal(t, "a", m.nodes[0].Ref.Branch) + assert.Equal(t, "b", m.nodes[1].Ref.Branch) + assert.Equal(t, "c", m.nodes[2].Ref.Branch) +} + +func TestInsertBlockedInReorderMode(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Reorder: move a down + m = sendKey(t, m, runeKey('J')) + assert.Equal(t, modeReorder, m.currentMode()) + + // Try insert below → should be blocked + m = sendKey(t, m, runeKey('i')) + assert.False(t, m.insertMode) + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "undo") + + // Try insert above → should be blocked + m = sendKey(t, m, runeKey('I')) + assert.False(t, m.insertMode) + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "undo") +} + +func TestReorderBlockedAfterInsert(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Insert + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + assert.Equal(t, modeStructure, m.currentMode()) + + // Try reorder → should be blocked + m = sendKey(t, m, runeKey('J')) + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "undo") +} + +func TestInsertCannotInsertOnMergedBranch(t *testing.T) { + nodes := []ModifyBranchNode{ + makeMergedNode("merged", 0), + makeNode("active", true, 1), + } + m := New(nodes, testTrunk, "1.0.0") + + // Force cursor to merged node + m.cursor = 0 + m = sendKey(t, m, runeKey('i')) + assert.False(t, m.insertMode) + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "merged") +} + +func TestInsertCancelledWithEscape(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + } + m := New(nodes, testTrunk, "1.0.0") + + // Start insert + m = sendKey(t, m, runeKey('i')) + require.True(t, m.insertMode) + + // Cancel with Escape + m = sendKey(t, m, tea.KeyMsg{Type: tea.KeyEscape}) + assert.False(t, m.insertMode) + assert.Len(t, m.nodes, 2, "no node should be added on cancel") +} + +func TestInsertEmptyNameCancels(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + } + m := New(nodes, testTrunk, "1.0.0") + + // Start insert, press Enter with empty input + m = sendKey(t, m, runeKey('i')) + require.True(t, m.insertMode) + m = sendKey(t, m, tea.KeyMsg{Type: tea.KeyEnter}) + assert.False(t, m.insertMode) + assert.Len(t, m.nodes, 2, "no node should be added on empty input") +} + +func TestInsertDuplicateNameBlocked(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + } + m := New(nodes, testTrunk, "1.0.0") + + // Try to insert with existing branch name "b" + m = sendKey(t, m, runeKey('i')) + require.True(t, m.insertMode) + m = simulateInsert(t, m, "b") + // Should still be in insert mode with an error + assert.True(t, m.insertMode, "should stay in insert mode on validation error") + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "already used") +} + +func TestInsertCountedInPendingSummary(t *testing.T) { + nodes := []ModifyBranchNode{ + { + BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "new"}}, + OriginalPosition: -1, + IsInserted: true, + PendingAction: &PendingAction{Type: ActionInsertBelow, NewName: "new"}, + }, + { + BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "b"}}, + OriginalPosition: 0, + }, + } + + result := pendingChangeSummary(nodes) + assert.Equal(t, "Pending: 1 insert", result) +} + +func TestInsertPluralInPendingSummary(t *testing.T) { + nodes := []ModifyBranchNode{ + { + BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "new1"}}, + OriginalPosition: -1, + IsInserted: true, + PendingAction: &PendingAction{Type: ActionInsertBelow, NewName: "new1"}, + }, + { + BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "b"}}, + OriginalPosition: 0, + }, + { + BranchNode: stackview.BranchNode{Ref: stack.BranchRef{Branch: "new2"}}, + OriginalPosition: -1, + IsInserted: true, + PendingAction: &PendingAction{Type: ActionInsertAbove, NewName: "new2"}, + }, + } + + result := pendingChangeSummary(nodes) + assert.Equal(t, "Pending: 2 inserts", result) +} + +func TestInsertMixedWithOtherActions(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Drop a + m = sendKey(t, m, runeKey('x')) + require.NotNil(t, m.nodes[0].PendingAction) + assert.Equal(t, ActionDrop, m.nodes[0].PendingAction.Type) + + // Move cursor to b and insert below it + m = sendKey(t, m, runeKey('j')) + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + + // Should now have 4 nodes: a(dropped), b, new-branch, c + require.Len(t, m.nodes, 4) + assert.Equal(t, modeStructure, m.currentMode()) + assert.Equal(t, ActionDrop, m.nodes[0].PendingAction.Type) + assert.Equal(t, "new-branch", m.nodes[2].Ref.Branch) + assert.True(t, m.nodes[2].IsInserted) +} + +func TestInsertOnRemovedBranchDoesNothing(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Drop a + m = sendKey(t, m, runeKey('x')) + require.True(t, m.nodes[0].Removed) + + // Force cursor back to removed node and try to insert + m.cursor = 0 + prevLen := len(m.nodes) + m = sendKey(t, m, runeKey('i')) + assert.False(t, m.insertMode, "should not enter insert mode on removed branch") + assert.Len(t, m.nodes, prevLen) +} + +func TestCurrentModeStructureAfterInsert(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + } + m := New(nodes, testTrunk, "1.0.0") + assert.Equal(t, modeNone, m.currentMode()) + + // Insert + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + assert.Equal(t, modeStructure, m.currentMode()) +} + +func TestApplyWithInsert(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + } + m := New(nodes, testTrunk, "1.0.0") + + // Insert a branch + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + + // Apply should be accepted + m = sendKey(t, m, tea.KeyMsg{Type: tea.KeyCtrlS}) + assert.True(t, m.applyRequested) +} + +func TestInsertAnnotation(t *testing.T) { + node := ModifyBranchNode{ + BranchNode: stackview.BranchNode{ + Ref: stack.BranchRef{Branch: "new-branch"}, + }, + PendingAction: &PendingAction{Type: ActionInsertBelow, NewName: "new-branch"}, + OriginalPosition: -1, + IsInserted: true, + } + + annotation := nodeAnnotation(node, 0) + require.NotNil(t, annotation) + assert.Equal(t, "✚ insert", annotation.Text) +} + +// --- Bug fix tests: insert should not cause false "moved" annotations --- + +func TestInsertDoesNotShowMovedAnnotation(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", false, 0), + makeNode("b", true, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Insert below b (cursor at 1) + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + + // Nodes are now: a(0), b(1), new-branch(inserted), c(2) + // Verify c does NOT have a "moved" annotation + require.Len(t, m.nodes, 4) + assert.Equal(t, "c", m.nodes[3].Ref.Branch) + + // Use effectiveIdx=2 for c (skipping the inserted node) + annotation := nodeAnnotation(m.nodes[3], 2) + assert.Nil(t, annotation, "c should not show a moved annotation after insert") + + // Also verify a and b have no annotation + assert.Nil(t, nodeAnnotation(m.nodes[0], 0), "a should have no annotation") + assert.Nil(t, nodeAnnotation(m.nodes[1], 1), "b should have no annotation") +} + +// --- Bug fix tests: header branch count excludes inserts --- + +func TestBranchCountExcludesInserts(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + m.width = 120 + m.height = 40 + + // Branch count should be 3 + cfg := m.buildHeaderConfig() + assert.Contains(t, cfg.InfoLines[1].Label, "3 branches") + + // Insert a branch + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + + // Branch count should still be 3 (not 4) + cfg = m.buildHeaderConfig() + assert.Contains(t, cfg.InfoLines[1].Label, "3 branches") +} + +// --- Bug fix tests: operations blocked on inserted nodes --- + +func TestCannotRenameInsertedBranch(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + } + m := New(nodes, testTrunk, "1.0.0") + + // Insert a branch and cursor lands on it + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + require.Equal(t, 1, m.cursor) + require.True(t, m.nodes[1].IsInserted) + + // Rename the inserted branch — should update the insert name directly + m = sendKey(t, m, runeKey('r')) + require.True(t, m.renameMode, "should enter rename mode on inserted branch") + + // Clear existing text and type new name + m = sendKey(t, m, tea.KeyMsg{Type: tea.KeyCtrlU}) // clear line + for _, r := range "better-name" { + m = sendKey(t, m, runeKey(r)) + } + m = sendKey(t, m, tea.KeyMsg{Type: tea.KeyEnter}) + assert.False(t, m.renameMode) + + // The node's branch name and insert action should be updated + assert.Equal(t, "better-name", m.nodes[1].Ref.Branch) + assert.True(t, m.nodes[1].IsInserted, "should still be marked as inserted") + assert.Equal(t, ActionInsertBelow, m.nodes[1].PendingAction.Type, "action type should still be insert") + assert.Equal(t, "better-name", m.nodes[1].PendingAction.NewName) + + // No separate rename action in the undo stack — only the original insert + for _, a := range m.actionStack { + assert.NotEqual(t, ActionRename, a.Type, "should not have a rename action in undo stack") + } +} + +func TestCannotFoldInsertedBranch(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + } + m := New(nodes, testTrunk, "1.0.0") + + // Insert a branch + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + require.True(t, m.nodes[1].IsInserted) + + // Try fold down → should be blocked + m = sendKey(t, m, runeKey('d')) + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "inserted") + + // Try fold up → should be blocked + m = sendKey(t, m, runeKey('u')) + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "inserted") +} + +func TestDropInsertedBranchRemovesIt(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Insert a branch below a + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + require.Len(t, m.nodes, 4) + require.True(t, m.nodes[1].IsInserted) + + // Drop (x) the inserted branch → should remove it entirely + m = sendKey(t, m, runeKey('x')) + assert.Len(t, m.nodes, 3, "inserted node should be removed") + assert.Equal(t, "a", m.nodes[0].Ref.Branch) + assert.Equal(t, "b", m.nodes[1].Ref.Branch) + assert.Equal(t, "c", m.nodes[2].Ref.Branch) +} + +func TestDropInsertedBranchCanBeUndone(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + } + m := New(nodes, testTrunk, "1.0.0") + + // Insert, then drop it + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + require.Len(t, m.nodes, 3) + + m = sendKey(t, m, runeKey('x')) + require.Len(t, m.nodes, 2) + require.Empty(t, m.actionStack, "undo stack should be empty after dropping an insert") + + // Dropping an inserted branch cancels the insert — undo stack should + // no longer contain the insert action. Pressing z undoes whatever came + // before the insert (i.e., nothing), not the drop itself. + m = sendKey(t, m, runeKey('z')) + assert.Len(t, m.nodes, 2, "undo should not re-insert — the drop cancelled the insert") + assert.Equal(t, "Nothing to undo", m.statusMessage) +} + +func TestCannotDropAllOriginalBranchesWithInsert(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + } + m := New(nodes, testTrunk, "1.0.0") + + // Insert a new branch + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + require.Len(t, m.nodes, 3) + + // Drop a + m.cursor = 0 + m = sendKey(t, m, runeKey('x')) + require.NotNil(t, m.nodes[0].PendingAction) + assert.Equal(t, ActionDrop, m.nodes[0].PendingAction.Type) + + // Move to b and try to drop it — should be refused since only + // the inserted branch would remain + m = sendKey(t, m, runeKey('j')) + m = sendKey(t, m, runeKey('j')) + require.Equal(t, "b", m.nodes[2].Ref.Branch) + m = sendKey(t, m, runeKey('x')) + assert.Nil(t, m.nodes[2].PendingAction, "should not be able to drop the last original branch") + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "last branch") +} + +func TestCannotFoldIntoInsertedBranch(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", true, 0), + makeNode("b", false, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Insert below a + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + // nodes: a(0), new-branch(1, inserted), b(2), c(3) + require.True(t, m.nodes[1].IsInserted) + + // Move cursor to a and try fold down — target would be the inserted branch + m.cursor = 0 + m = sendKey(t, m, runeKey('d')) + assert.Nil(t, m.nodes[0].PendingAction, "should not fold into an inserted branch") + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "inserted") +} + +func TestCannotFoldUpIntoInsertedBranch(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", false, 0), + makeNode("b", true, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Insert above b (at index 1) + m = sendKey(t, m, runeKey('I')) + m = simulateInsert(t, m, "new-branch") + // nodes: a(0), new-branch(1, inserted), b(2), c(3) + require.True(t, m.nodes[1].IsInserted) + + // Move cursor to b and try fold up — target would be the inserted branch + m.cursor = 2 + m = sendKey(t, m, runeKey('u')) + assert.Nil(t, m.nodes[2].PendingAction, "should not fold up into an inserted branch") + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "inserted") +} + +func TestDropBlockedWhenFoldWouldRetargetToInserted(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", false, 0), + makeNode("b", true, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Fold a down into b + m.cursor = 0 + m = sendKey(t, m, runeKey('d')) + require.NotNil(t, m.nodes[0].PendingAction) + assert.Equal(t, ActionFoldDown, m.nodes[0].PendingAction.Type) + + // Insert between b and c + m.cursor = 1 + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + // nodes: a(0, fold-down), b(1), new-branch(2, inserted), c(3) + + // Try to drop b — fold-down on a currently targets b. + // If b is dropped, fold target shifts to new-branch (inserted) → block + m.cursor = 1 + m = sendKey(t, m, runeKey('x')) + assert.Nil(t, m.nodes[1].PendingAction, "drop should be blocked") + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "fold") +} + +func TestCannotMutualFold(t *testing.T) { + // Scenario: a folds down into b, then try to fold b up. + // Since a is Removed after folding, b's fold-up target skips a. + // But b is the target of a's fold — so b should not be allowed to fold. + nodes := []ModifyBranchNode{ + makeNode("a", false, 0), + makeNode("b", true, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Fold a down (target = b) + m.cursor = 0 + m = sendKey(t, m, runeKey('d')) + require.NotNil(t, m.nodes[0].PendingAction) + assert.Equal(t, ActionFoldDown, m.nodes[0].PendingAction.Type) + + // Move to b — b is the target of a's fold. + // Try to fold b up — should be blocked because a is folding into b. + m.cursor = 1 + m = sendKey(t, m, runeKey('u')) + assert.Nil(t, m.nodes[1].PendingAction, "should not fold a branch that is receiving a fold") + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "folding into this branch") +} + +func TestCannotMutualFold_FourNodes(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", false, 0), + makeNode("b", false, 1), + makeNode("c", true, 2), + makeNode("d", false, 3), + } + m := New(nodes, testTrunk, "1.0.0") + + // Fold c down into d + m.cursor = 2 + m = sendKey(t, m, runeKey('d')) + require.NotNil(t, m.nodes[2].PendingAction) + assert.Equal(t, ActionFoldDown, m.nodes[2].PendingAction.Type) + + // Now try fold d up — d is c's fold target, so d can't fold + m.cursor = 3 + m = sendKey(t, m, runeKey('u')) + assert.Nil(t, m.nodes[3].PendingAction, "should not fold a branch that is receiving a fold") + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "folding into this branch") + + // But folding d down should still work (it's not mutual) + m = sendKey(t, m, runeKey('d')) + // d has no branch below, so this should fail with "No branch below" + assert.True(t, m.statusIsError) +} + +func TestCannotFoldTargetOfFold(t *testing.T) { + // b folds up into a, then try to fold a down — a is receiving b's fold + nodes := []ModifyBranchNode{ + makeNode("a", false, 0), + makeNode("b", true, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Fold b up into a + m.cursor = 1 + m = sendKey(t, m, runeKey('u')) + require.NotNil(t, m.nodes[1].PendingAction) + assert.Equal(t, ActionFoldUp, m.nodes[1].PendingAction.Type) + + // Try to fold a down — a is b's fold target, should be blocked + m.cursor = 0 + m = sendKey(t, m, runeKey('d')) + assert.Nil(t, m.nodes[0].PendingAction, "should not fold a branch that is receiving a fold") + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "folding into this branch") +} + +func TestInsertBlockedBetweenFoldDownAndTarget(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", false, 0), + makeNode("b", true, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Fold a down into b + m.cursor = 0 + m = sendKey(t, m, runeKey('d')) + require.NotNil(t, m.nodes[0].PendingAction) + assert.Equal(t, ActionFoldDown, m.nodes[0].PendingAction.Type) + + // Try inserting above b (between a and b) — should be blocked immediately + m.cursor = 1 + m = sendKey(t, m, runeKey('I')) + assert.False(t, m.insertMode, "should not enter insert mode") + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "folding") +} + +func TestInsertBlockedBetweenFoldUpAndTarget(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", false, 0), + makeNode("b", true, 1), + makeNode("c", false, 2), + } + m := New(nodes, testTrunk, "1.0.0") + + // Fold b up into a + m.cursor = 1 + m = sendKey(t, m, runeKey('u')) + require.NotNil(t, m.nodes[1].PendingAction) + assert.Equal(t, ActionFoldUp, m.nodes[1].PendingAction.Type) + + // Try inserting below a (between a and b) — should be blocked immediately + m.cursor = 0 + m = sendKey(t, m, runeKey('i')) + assert.False(t, m.insertMode, "should not enter insert mode") + assert.True(t, m.statusIsError) + assert.Contains(t, m.statusMessage, "folding") +} + +func TestInsertAllowedOutsideFoldRange(t *testing.T) { + nodes := []ModifyBranchNode{ + makeNode("a", false, 0), + makeNode("b", true, 1), + makeNode("c", false, 2), + makeNode("d", false, 3), + } + m := New(nodes, testTrunk, "1.0.0") + + // Fold a down into b + m.cursor = 0 + m = sendKey(t, m, runeKey('d')) + require.NotNil(t, m.nodes[0].PendingAction) + + // Inserting below c (between c and d) should be fine — outside fold range + m.cursor = 2 + m = sendKey(t, m, runeKey('i')) + m = simulateInsert(t, m, "new-branch") + assert.False(t, m.insertMode, "should succeed") + assert.Len(t, m.nodes, 5) + assert.Equal(t, "new-branch", m.nodes[3].Ref.Branch) +} diff --git a/internal/tui/modifyview/status.go b/internal/tui/modifyview/status.go index cb5d489..5f76c44 100644 --- a/internal/tui/modifyview/status.go +++ b/internal/tui/modifyview/status.go @@ -10,7 +10,7 @@ import ( // pendingChangeSummary returns a summary string of all pending changes. // E.g. "Pending: 1 drop, 1 fold, 2 moves, 1 rename" func pendingChangeSummary(nodes []ModifyBranchNode) string { - var drops, foldDowns, foldUps, moves, renames int + var drops, foldDowns, foldUps, moves, renames, inserts int for _, n := range nodes { if n.PendingAction == nil { @@ -27,17 +27,25 @@ func pendingChangeSummary(nodes []ModifyBranchNode) string { moves++ case ActionRename: renames++ + case ActionInsertBelow, ActionInsertAbove: + inserts++ } } - // Also count nodes that have moved from their original position - for i, n := range nodes { - if !n.Removed && n.PendingAction == nil && n.OriginalPosition != i { + // Also count nodes that have moved from their original position. + // Skip inserted nodes — they shift indices of other nodes. + effectiveIdx := 0 + for _, n := range nodes { + if n.IsInserted { + continue + } + if !n.Removed && n.PendingAction == nil && n.OriginalPosition != effectiveIdx { moves++ } + effectiveIdx++ } - if drops == 0 && foldDowns == 0 && foldUps == 0 && moves == 0 && renames == 0 { + if drops == 0 && foldDowns == 0 && foldUps == 0 && moves == 0 && renames == 0 && inserts == 0 { return "" } @@ -49,6 +57,9 @@ func pendingChangeSummary(nodes []ModifyBranchNode) string { if folds > 0 { parts = append(parts, fmt.Sprintf("%d %s", folds, pluralize(folds, "fold", "folds"))) } + if inserts > 0 { + parts = append(parts, fmt.Sprintf("%d %s", inserts, pluralize(inserts, "insert", "inserts"))) + } if moves > 0 { parts = append(parts, fmt.Sprintf("%d %s", moves, pluralize(moves, "move", "moves"))) } diff --git a/internal/tui/modifyview/styles.go b/internal/tui/modifyview/styles.go index 2c7b9d9..4435902 100644 --- a/internal/tui/modifyview/styles.go +++ b/internal/tui/modifyview/styles.go @@ -8,15 +8,18 @@ var ( foldBadge = lipgloss.NewStyle().Foreground(lipgloss.Color("3")) // yellow renameBadge = lipgloss.NewStyle().Foreground(lipgloss.Color("14")) // cyan moveBadge = lipgloss.NewStyle().Foreground(lipgloss.Color("5")) // magenta/purple + insertBadge = lipgloss.NewStyle().Foreground(lipgloss.Color("2")) // green - // Branch name overrides for drop/fold - dropBranchStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("1")).Strikethrough(true) // red strikethrough - foldBranchStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("3")).Strikethrough(true) // yellow strikethrough + // Branch name overrides for drop/fold/insert + dropBranchStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("1")).Strikethrough(true) // red strikethrough + foldBranchStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("3")).Strikethrough(true) // yellow strikethrough + insertBranchStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("2")) // green - // Connector color overrides for drop/fold/move - dropConnectorStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("1")) // red - foldConnectorStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("3")) // yellow - movedConnectorStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("5")) // magenta/purple + // Connector color overrides for drop/fold/move/insert + dropConnectorStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("1")) // red + foldConnectorStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("3")) // yellow + movedConnectorStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("5")) // magenta/purple + insertConnectorStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("2")) // green // Status line styles statusBarStyle = lipgloss.NewStyle().Foreground(lipgloss.Color("8")) diff --git a/internal/tui/modifyview/types.go b/internal/tui/modifyview/types.go index 4598c6b..2d8de63 100644 --- a/internal/tui/modifyview/types.go +++ b/internal/tui/modifyview/types.go @@ -8,11 +8,13 @@ import ( type ActionType string const ( - ActionDrop ActionType = "drop" - ActionFoldDown ActionType = "fold_down" - ActionFoldUp ActionType = "fold_up" - ActionMove ActionType = "move" - ActionRename ActionType = "rename" + ActionDrop ActionType = "drop" + ActionFoldDown ActionType = "fold_down" + ActionFoldUp ActionType = "fold_up" + ActionMove ActionType = "move" + ActionRename ActionType = "rename" + ActionInsertBelow ActionType = "insert_below" + ActionInsertAbove ActionType = "insert_above" ) // PendingAction represents a staged modification on a branch. @@ -39,16 +41,18 @@ type ModifyBranchNode struct { PendingAction *PendingAction OriginalPosition int Removed bool // true if this branch has been dropped or folded + IsInserted bool // true if this branch was inserted during modify (not yet in git) } // ApplyResult holds the outcome of applying modifications. type ApplyResult struct { - Success bool - DroppedPRs []DroppedPR - RenamedBranches []RenamedBranch - MovedBranches int - NeedsSubmit bool // true when the modify affected branches with PRs - Message string + Success bool + DroppedPRs []DroppedPR + RenamedBranches []RenamedBranch + InsertedBranches []string + MovedBranches int + NeedsSubmit bool // true when the modify affected branches with PRs + Message string } // DroppedPR records a PR that was dropped from the stack. diff --git a/internal/tui/shared/header.go b/internal/tui/shared/header.go index d686e3f..edec801 100644 --- a/internal/tui/shared/header.go +++ b/internal/tui/shared/header.go @@ -16,10 +16,10 @@ const MinHeightForHeader = 25 const MinWidthForShortcuts = 65 // MinWidthForHeader is the minimum width to show the header at all. -const MinWidthForHeader = 50 +const MinWidthForHeader = 53 // MinWidthForArt is the minimum width to show ASCII art in the header. -const MinWidthForArt = 88 +const MinWidthForArt = 96 // ShortcutEntry represents a keyboard shortcut for the header. type ShortcutEntry struct {