Bring uncommitted changes along when sync moves you off a merged branch

README.md

@@ -339,6 +339,10 @@ lock on the branch itself (`.git/refs/heads/main.lock`, which can happen whether
 checked out) is reported straight away with its path rather than waited on, because by then
 git may already have updated the checkout. It never deletes a lock itself.
 
+If the branch being deleted is the one you're on, sync switches you to its trunk first, and like
+`git switch` your uncommitted changes come along. It only keeps the branch when git says the
+switch would overwrite them, and then names the files.
+
 Flags:
 - `-d` / `--delete-all` delete merged or closed branches without asking
 - `-f` / `--force` like `-d`, and also reset a diverged trunk to the remote

docs/architecture.md

@@ -270,7 +270,9 @@ A branch recorded in two worktrees' metadata files is read from the first; once
 deleted the stale record in the other file shows up as gone on the next sync and is
 dropped then, so it heals over two syncs. A candidate checked out in the current worktree whose
 trunk is checked out in another worktree leaves HEAD detached at the trunk tip rather than
-failing. Consent for deletion is `stack.sync.prune` (`always` by default), `-d`, or `-f`;
+failing. Moving the current checkout off a branch being deleted is a plain `git switch`, so
+uncommitted changes git can carry come along; only git's "would be overwritten" refusal keeps
+the branch (as `dirty`, files named); any other failure stops the sync as before. Consent for deletion is `stack.sync.prune` (`always` by default), `-d`, or `-f`;
 with no terminal and `ask`, branches are kept with a notice. Nothing is pushed; `submit`
 does that.
 

pkg/app/sync_cleanup.go

@@ -87,27 +87,18 @@ func (a *App) deleteApproved(ctx context.Context, st *syncState, approved []cand
 		switch lb.Worktree {
 		case "":
 		case st.repo.TopLevel:
-			if st.isDirty(ctx, a.d.Git, lb.Worktree) {
-				res.Kept = append(res.Kept, KeptBranch{Name: c.name, Reason: KeptDirty})
-				res.notice("%s is checked out here with uncommitted changes, so it was kept; commit or stash them and sync again", c.name)
-				continue
-			}
 			trunk, ok := st.local[c.trunk]
 			if !ok {
 				res.Kept = append(res.Kept, KeptBranch{Name: c.name, Reason: KeptNoTrunk})
 				res.notice("%s is checked out here and its trunk %s has no local branch, so it was kept", c.name, c.trunk)
 				continue
 			}
-			if trunk.Worktree != "" && trunk.Worktree != st.repo.TopLevel {
-				if err := a.d.Git.SwitchDetached(ctx, st.repo, trunk.Head); err != nil {
-					return err
-				}
-				res.notice("%s was checked out here; HEAD is now detached at %s (%s) because %s is checked out in %s", c.name, c.trunk, short(trunk.Head), c.trunk, shortPath(trunk.Worktree))
-			} else {
-				if err := a.d.Git.Switch(ctx, st.repo, c.trunk); err != nil {
-					return err
-				}
-				st.local[c.trunk] = withWorktree(trunk, st.repo.TopLevel)
+			moved, err := a.moveOffDeleted(ctx, st, c, trunk, res)
+			if err != nil {
+				return err
+			}
+			if !moved {
+				continue
 			}
 		default:
 			res.Kept = append(res.Kept, KeptBranch{Name: c.name, Reason: KeptOtherWorktree})
@@ -124,6 +115,58 @@ func (a *App) deleteApproved(ctx context.Context, st *syncState, approved []cand
 	return nil
 }
 
+// moveOffDeleted switches the current checkout from a branch about to be
+// deleted to its trunk (or to the trunk's tip, detached, when the trunk is
+// checked out in another worktree). Uncommitted changes, untracked files
+// included, come along the way git switch carries them; only when git
+// refuses because they'd be overwritten is the branch kept and false
+// returned. Any other failure is returned as is.
+func (a *App) moveOffDeleted(ctx context.Context, st *syncState, c candidate, trunk git.Branch, res *SyncResult) (bool, error) {
+	dirty := st.isDirty(ctx, a.d.Git, st.repo.TopLevel)
+	if !dirty {
+		dirty, _ = a.d.Git.HasUntrackedFiles(ctx, st.repo)
+	}
+	detach := trunk.Worktree != "" && trunk.Worktree != st.repo.TopLevel
+	var err error
+	if detach {
+		err = a.d.Git.SwitchDetached(ctx, st.repo, trunk.Head)
+	} else {
+		err = a.d.Git.Switch(ctx, st.repo, c.trunk)
+	}
+	if oe, ok := errors.AsType[*git.OverwriteError](err); ok {
+		var what []string
+		if len(oe.Changed) > 0 {
+			what = append(what, "your changes to "+joinNames(oe.Changed))
+		}
+		step := "commit or stash them and sync again"
+		if len(oe.Untracked) > 0 {
+			what = append(what, "untracked "+joinNames(oe.Untracked))
+			step = "commit them, move them aside or stash them with git stash --include-untracked, then sync again"
+		}
+		res.Kept = append(res.Kept, KeptBranch{Name: c.name, Reason: KeptDirty})
+		res.notice("%s is checked out here and switching to %s would overwrite %s, so it was kept; %s",
+			c.name, c.trunk, strings.Join(what, " and "), step)
+		return false, nil
+	}
+	if err != nil {
+		return false, err
+	}
+	carried := ""
+	if dirty {
+		carried = "; your uncommitted changes came along"
+	}
+	if detach {
+		res.notice("%s was checked out here; HEAD is now detached at %s (%s) because %s is checked out in %s%s",
+			c.name, c.trunk, short(trunk.Head), c.trunk, shortPath(trunk.Worktree), carried)
+	} else {
+		st.local[c.trunk] = withWorktree(trunk, st.repo.TopLevel)
+		if dirty {
+			res.notice("%s was checked out here, so you are now on %s%s", c.name, c.trunk, carried)
+		}
+	}
+	return true, nil
+}
+
 // short abbreviates a commit id for messages.
 func short(sha string) string {
 	if len(sha) > 7 {

pkg/app/sync_cleanup_test.go

@@ -336,10 +336,70 @@ func TestSyncMovesCurrentWorktreeOffDeletedBranch(t *testing.T) {
 	}
 }
 
-func TestSyncKeepsDeletedBranchWhenCurrentTreeIsDirty(t *testing.T) {
+func TestSyncCarriesUncommittedChangesToTrunk(t *testing.T) {
 	f := newSyncFixture(t)
 	gittest.Run(t, f.dir, "switch", "-q", "a")
 	f.mergeOnRemote(t, "a", 0)
+	// a.txt is the same on a and on the merged main, so git can carry the edit over.
+	gittest.WriteFile(t, f.dir, "a.txt", "edited")
+	res, err := f.sync(t, app.SyncOptions{NoRestack: true})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, ok := deleted(res, "a"); !ok || branchExists(t, f.dir, "a") {
+		t.Fatalf("a should be deleted: deleted = %+v kept = %+v", res.Deleted, res.Kept)
+	}
+	if cur := gittest.Run(t, f.dir, "branch", "--show-current"); cur != "main" {
+		t.Errorf("on %s, want main", cur)
+	}
+	if got := gittest.Run(t, f.dir, "status", "--porcelain"); got != "M a.txt" {
+		t.Errorf("status = %q; the edit should have come along", got)
+	}
+	if !strings.Contains(strings.Join(res.Notices, "\n"), "uncommitted changes came along") {
+		t.Errorf("notices = %v", res.Notices)
+	}
+}
+
+func TestSyncSaysWhenOnlyUntrackedFilesCameAlong(t *testing.T) {
+	f := newSyncFixture(t)
+	gittest.Run(t, f.dir, "switch", "-q", "a")
+	f.mergeOnRemote(t, "a", 0)
+	gittest.WriteFile(t, f.dir, "notes.txt", "mine")
+	res, err := f.sync(t, app.SyncOptions{NoRestack: true})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, ok := deleted(res, "a"); !ok {
+		t.Fatalf("a should be deleted: kept = %+v", res.Kept)
+	}
+	if !strings.Contains(strings.Join(res.Notices, "\n"), "uncommitted changes came along") {
+		t.Errorf("notices = %v", res.Notices)
+	}
+}
+
+func TestSyncKeepsDeletedBranchWhenSwitchWouldOverwriteUntrackedFiles(t *testing.T) {
+	f := newSyncFixture(t)
+	gittest.Run(t, f.dir, "switch", "-q", "a")
+	f.mergeOnRemote(t, "a", 0)
+	f.advanceRemote(t, "main", "new.txt") // main gains a file a doesn't have
+	gittest.WriteFile(t, f.dir, "new.txt", "mine")
+	res, err := f.sync(t, app.SyncOptions{NoRestack: true})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if k, ok := kept(res, "a"); !ok || k.Reason != app.KeptDirty {
+		t.Errorf("kept = %+v", res.Kept)
+	}
+	if n := strings.Join(res.Notices, "\n"); !strings.Contains(n, "untracked new.txt") || !strings.Contains(n, "--include-untracked") {
+		t.Errorf("notices = %v", res.Notices)
+	}
+}
+
+func TestSyncKeepsDeletedBranchWhenSwitchWouldOverwriteChanges(t *testing.T) {
+	f := newSyncFixture(t)
+	gittest.Run(t, f.dir, "switch", "-q", "a")
+	f.mergeOnRemote(t, "a", 0)
+	f.advanceRemote(t, "main", "a.txt") // now a.txt differs between a and main
 	gittest.WriteFile(t, f.dir, "a.txt", "edited")
 	res, err := f.sync(t, app.SyncOptions{NoRestack: true})
 	if err != nil {
@@ -348,6 +408,12 @@ func TestSyncKeepsDeletedBranchWhenCurrentTreeIsDirty(t *testing.T) {
 	if k, ok := kept(res, "a"); !ok || k.Reason != app.KeptDirty || !branchExists(t, f.dir, "a") {
 		t.Errorf("kept = %+v", res.Kept)
 	}
+	if cur := gittest.Run(t, f.dir, "branch", "--show-current"); cur != "a" {
+		t.Errorf("on %s, want a", cur)
+	}
+	if !strings.Contains(strings.Join(res.Notices, "\n"), "would overwrite your changes to a.txt") {
+		t.Errorf("notices = %v", res.Notices)
+	}
 }
 
 func TestSyncDropsGoneBranchesFromMetadata(t *testing.T) {

pkg/git/branch.go

@@ -72,6 +72,9 @@ func (c *Client) Switch(ctx context.Context, repo Repo, name string) error {
 	_, err := c.gitIn(ctx, repo, "switch", "--no-guess", name)
 	if err != nil {
 		if ee, ok := errors.AsType[*exec.ExitError](err); ok {
+			if oe := overwriteError(ee); oe != nil {
+				return oe
+			}
 			return fmt.Errorf("switch to %q: %s", name, ee.Result.Err())
 		}
 		return err
@@ -124,6 +127,9 @@ func (c *Client) SwitchDetached(ctx context.Context, repo Repo, rev string) erro
 	_, err := c.gitIn(ctx, repo, "switch", "--detach", rev)
 	if err != nil {
 		if ee, ok := errors.AsType[*exec.ExitError](err); ok {
+			if oe := overwriteError(ee); oe != nil {
+				return oe
+			}
 			return fmt.Errorf("switch --detach %q: %s", rev, ee.Result.Err())
 		}
 		return err

pkg/git/refs.go

@@ -103,8 +103,8 @@ func (c *Client) MergeFF(ctx context.Context, repo Repo, rev string) error {
 	_, err := c.gitIn(ctx, repo, "merge", "--ff-only", "--quiet", rev)
 	if err != nil {
 		if ee, ok := errors.AsType[*exec.ExitError](err); ok && LockPath(err) == "" {
-			if changed, untracked := parseOverwrite(ee.Result.Err()); len(changed)+len(untracked) > 0 {
-				return &OverwriteError{Changed: changed, Untracked: untracked}
+			if oe := overwriteError(ee); oe != nil {
+				return oe
 			}
 			return fmt.Errorf("fast forward: %s", firstLine(ee.Result.Err()))
 		}
@@ -197,7 +197,16 @@ func (e *OverwriteError) Error() string {
 	if n := len(e.Untracked); n > 0 {
 		parts = append(parts, fmt.Sprintf("%d untracked", n))
 	}
-	return "fast forward would overwrite " + strings.Join(parts, " and ") + " files"
+	return "moving the checkout would overwrite " + strings.Join(parts, " and ") + " files"
+}
+
+// overwriteError is the *OverwriteError in a git failure, or nil when git
+// failed for some other reason.
+func overwriteError(ee *exec.ExitError) *OverwriteError {
+	if changed, untracked := parseOverwrite(ee.Result.Err()); len(changed)+len(untracked) > 0 {
+		return &OverwriteError{Changed: changed, Untracked: untracked}
+	}
+	return nil
 }
 
 // parseOverwrite reads the file lists out of git's "would be overwritten"

pkg/git/worktree_test.go

@@ -2,9 +2,11 @@ package git_test
 
 import (
 	"context"
+	"errors"
 	"path/filepath"
 	"testing"
 
+	"github.com/DomBlack/git-stack/pkg/git"
 	"github.com/DomBlack/git-stack/pkg/git/gittest"
 )
 
@@ -39,3 +41,32 @@ func TestWorktrees(t *testing.T) {
 		t.Errorf("detached worktree = %+v", wts[2])
 	}
 }
+
+func TestSwitchReportsFilesInTheWay(t *testing.T) {
+	gittest.Isolate(t)
+	dir := gittest.InitRepo(t)
+	c := newClient()
+	ctx := context.Background()
+	repo, err := c.Discover(ctx, dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	gittest.Run(t, dir, "switch", "-q", "-c", "side")
+	gittest.Commit(t, dir, "f.txt", "side", "side changes f")
+	gittest.WriteFile(t, dir, "f.txt", "edited")
+	gittest.WriteFile(t, dir, "other.txt", "carried")
+
+	err = c.Switch(ctx, repo, "main")
+	oe, ok := errors.AsType[*git.OverwriteError](err)
+	if !ok || len(oe.Changed) != 1 || oe.Changed[0] != "f.txt" {
+		t.Fatalf("err = %v; want an OverwriteError naming f.txt", err)
+	}
+
+	gittest.Run(t, dir, "checkout", "-q", "--", "f.txt")
+	if err := c.Switch(ctx, repo, "main"); err != nil {
+		t.Fatalf("a change git can carry over should not stop the switch: %v", err)
+	}
+	if got := gittest.Run(t, dir, "status", "--porcelain"); got != "?? other.txt" {
+		t.Errorf("status = %q", got)
+	}
+}