diff --git a/pkg/absorb/patch.go b/pkg/absorb/patch.go index 8f43939..201718d 100644 --- a/pkg/absorb/patch.go +++ b/pkg/absorb/patch.go @@ -10,8 +10,13 @@ import ( // Patch renders hunks for one file as a zero context patch for git apply, // the path on both sides. The hunks are sorted by old line (an insertion // after a line goes after an edit of that line) and their new side -// numbers are recomputed so each one accounts for the ones before it, -// which is what git apply uses to place an insertion. +// numbers are recomputed so each one accounts for the ones before it. +// git apply takes a hunk's new start as the line, in the file with the +// hunks above already applied, where its removed lines begin (or before +// which its added lines go), and reports an offset when they are anywhere +// else. That is git diff's numbering except for a deletion, where git diff +// gives the line before the gap; here a deletion names its first removed +// line instead, so a correct patch applies exactly where it says. func Patch(path string, hunks []Hunk) string { sorted := slices.Clone(hunks) slices.SortStableFunc(sorted, func(a, b Hunk) int { @@ -28,11 +33,8 @@ func Patch(path string, hunks []Hunk) string { delta := 0 for _, h := range sorted { newStart := h.OldStart + delta - switch { - case h.OldCount == 0: + if h.OldCount == 0 { newStart++ - case h.NewCount == 0: - newStart-- } fmt.Fprintf(&b, "@@ -%d,%d +%d,%d @@\n", h.OldStart, h.OldCount, newStart, h.NewCount) for _, l := range h.Lines { diff --git a/pkg/absorb/patch_test.go b/pkg/absorb/patch_test.go index 78c1b65..4c6c6f5 100644 --- a/pkg/absorb/patch_test.go +++ b/pkg/absorb/patch_test.go @@ -19,7 +19,7 @@ func TestPatchRenumbersCumulatively(t *testing.T) { "@@ -1,0 +2,2 @@\n+P\n+Q\n" + "@@ -3,0 +6,1 @@\n+R\n" + "@@ -5,1 +8,1 @@\n-l5\n+L5\n" + - "@@ -8,2 +10,0 @@\n-l8\n-l9\n" + "@@ -8,2 +11,0 @@\n-l8\n-l9\n" if got := Patch("f.txt", hunks); got != want { t.Errorf("got:\n%s\nwant:\n%s", got, want) } diff --git a/pkg/app/absorb_test.go b/pkg/app/absorb_test.go index 653a700..b013b06 100644 --- a/pkg/app/absorb_test.go +++ b/pkg/app/absorb_test.go @@ -660,3 +660,118 @@ func TestAbsorbRestackRefusalAboveSaysWhatIsLeft(t *testing.T) { t.Error("c should be untouched") } } + +// commitOnA adds a commit writing path on a and moves b and c on top of it, +// metadata included. +func (f *absorbFixture) commitOnA(t *testing.T, path, content, msg string) { + t.Helper() + gittest.Run(t, f.dir, "switch", "-q", "a") + gittest.Commit(t, f.dir, path, content, msg) + gittest.Run(t, f.dir, "switch", "-q", "b") + gittest.Run(t, f.dir, "rebase", "-q", "a") + gittest.Run(t, f.dir, "switch", "-q", "c") + gittest.Run(t, f.dir, "rebase", "-q", "b") + gittest.Run(t, f.dir, "switch", "-q", "b") + br := f.fb.graph.Stacks[0].Branches + br[0].Head, br[1].Head, br[1].Base, br[2].Head, br[2].Base = f.rev(t, "a"), f.rev(t, "b"), f.rev(t, "a"), f.rev(t, "c"), f.rev(t, "b") +} + +// lines joins ls into file content, one per line. +func lines(ls ...string) string { return strings.Join(ls, "\n") + "\n" } + +// span is the numbered lines from to to as strings. +func span(from, to int) []string { + var out []string + for i := from; i <= to; i++ { + out = append(out, fmt.Sprint(i)) + } + return out +} + +// Deletions after other hunks of the same file shift the lines below them, +// so each hunk has to say where it lands once the ones above it are in. +func TestAbsorbManyHunksIntoOneCommitOfOneFile(t *testing.T) { + f := newAbsorbFixture(t) + f.commitOnA(t, "g.txt", lines(span(1, 30)...), "feat: g") + + want := lines(slices.Concat( + span(1, 3), []string{"new", "lines"}, span(4, 5), // insert after 3 + span(9, 11), []string{"twelve"}, // delete 6-8, edit 12 + span(13, 14), span(16, 20), []string{"x"}, // delete 15, insert after 20 + span(21, 24), []string{"y"}, span(27, 30), // 25 and 26 become y + )...) + gittest.WriteFile(t, f.dir, "g.txt", want) + gittest.Run(t, f.dir, "add", "g.txt") + res, err := f.app.Absorb(context.Background(), f.repo, app.AbsorbOptions{}) + if err != nil || res.Plan.Absorbed != 6 || res.Plan.Unabsorbed != 0 || res.Plan.Commits != 1 { + t.Fatalf("res = %+v %v", res, err) + } + for _, rev := range []string{"a", "b", "c"} { + if got := gittest.Run(t, f.dir, "show", rev+":g.txt") + "\n"; got != want { + t.Errorf("%s:g.txt = %q, want %q", rev, got, want) + } + } + if _, err := gittest.RunErr(t, f.dir, "diff", "--cached", "--quiet"); err != nil { + t.Error("index should be clean") + } +} + +// Hunks of one file going to two commits, interleaved, several to each. +func TestAbsorbHunksOfOneFileIntoTwoCommits(t *testing.T) { + f := newAbsorbFixture(t) + f.commitOnA(t, "g.txt", lines(span(1, 30)...), "feat: g") + bVersion := lines(slices.Concat(span(1, 7), []string{"b8"}, span(9, 21), []string{"b22"}, span(23, 30))...) + gittest.Commit(t, f.dir, "g.txt", bVersion, "feat: b edits g") + br := f.fb.graph.Stacks[0].Branches + br[1].Head = f.rev(t, "b") + gittest.Run(t, f.dir, "switch", "-q", "c") + gittest.Run(t, f.dir, "rebase", "-q", "b") + gittest.Run(t, f.dir, "switch", "-q", "b") + br[2].Head, br[2].Base = f.rev(t, "c"), f.rev(t, "b") + + staged := lines(slices.Concat( + span(1, 2), span(5, 7), // delete 3 and 4: a + []string{"B8", "B8.5"}, // b8 grows a line: b + span(9, 11), span(13, 15), []string{"x", "y", "z"}, // delete 12, insert after 15: a + span(16, 21), span(24, 26), // delete b22 and 23: b + []string{"TWENTY-SEVEN"}, span(28, 30), // edit 27: a + )...) + gittest.WriteFile(t, f.dir, "g.txt", staged) + gittest.Run(t, f.dir, "add", "g.txt") + res, err := f.app.Absorb(context.Background(), f.repo, app.AbsorbOptions{}) + if err != nil { + t.Fatalf("res = %+v %v", res, err) + } + if got := targetsOf(res.Plan); !slices.Equal(got, []string{"a", "b", "a", "a", "b", "a"}) { + t.Errorf("targets = %v (hunks %+v)", got, res.Plan.Hunks) + } + wantA := lines(slices.Concat(span(1, 2), span(5, 11), span(13, 15), []string{"x", "y", "z"}, span(16, 26), []string{"TWENTY-SEVEN"}, span(28, 30))...) + for _, rev := range []string{"a"} { + if got := gittest.Run(t, f.dir, "show", rev+":g.txt") + "\n"; got != wantA { + t.Errorf("%s:g.txt = %q, want %q", rev, got, wantA) + } + } + if got := gittest.Run(t, f.dir, "show", "b:g.txt") + "\n"; got != staged { + t.Errorf("b:g.txt = %q, want %q", got, staged) + } + if _, err := gittest.RunErr(t, f.dir, "diff", "--cached", "--quiet"); err != nil { + t.Error("index should be clean") + } +} + +// A lone deletion has nothing above it shifting lines, and still has to +// apply exactly where it says. +func TestAbsorbLoneDeletion(t *testing.T) { + f := newAbsorbFixture(t) + f.stageLine(t, 15, "") + cur := gittest.Run(t, f.dir, "show", ":f.txt") + "\n" + gittest.WriteFile(t, f.dir, "f.txt", strings.Replace(cur, "\n\n", "\n", 1)) + gittest.Run(t, f.dir, "add", "f.txt") + res, err := f.app.Absorb(context.Background(), f.repo, app.AbsorbOptions{}) + if err != nil || res.Plan.Absorbed != 1 || len(res.Rewritten) != 1 || res.Rewritten[0].Name != "b" { + t.Fatalf("res = %+v %v", res, err) + } + if got := gittest.Run(t, f.dir, "show", "b:f.txt"); strings.Contains(got, "fifteen") || !strings.Contains(got, "\n14\n16\n") { + t.Errorf("b:f.txt = %q", got) + } +}