Take lines back out of the custom patch from the pane showing it

Space in the pane previewing the patch had nothing to act on: everything
there is in the patch already, so it can only mean taking those lines back
out, in the way that space in the staged side of the working tree's diff
only unstages.

A line of the patch can't be found by its number: the patch renumbers
whatever follows a change it leaves out, so a line of it names a line of
the commit's diff that isn't the one shown. A line is identified instead
by its position among its file's changes, counted in the diff the patch is
shown as. That is the same position it has among the changes the patch
holds for the file, since both are in the order the file has them. This
holds however the rendering came out, so a renderer that groups a hunk's
deletions before its additions, or leaves a change out altogether, no
longer matters.

The pane's rows are stated in terms of the trees the patch was
materialized into, so the paths come back to the repo's own before
anything is looked up in them. This also lets the lines there be copied
and edited, as the lines of a commit's diff can be.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

pkg/commands/patch/patch_builder.go

@@ -473,6 +473,29 @@ func (p *PatchBuilder) IncludedLineIdentities(filename string) []LineIdentity {
 	return identities
 }
 
+// IncludedChangeLineIndices says which of filename's change lines are in the patch, as
+// their indices in the file's diff and in the order the file has them.
+//
+// It is how a line of the patch as it is shown names the line of the diff it came from:
+// all that can be said about a line of the patch is which of the file's changes it is,
+// its line numbers being the patch's own — a patch that leaves an earlier addition out
+// numbers everything after it differently from the diff it was built from.
+func (p *PatchBuilder) IncludedChangeLineIndices(filename string) []int {
+	info, ok := p.snapshotFileInfoMap()[filename]
+	if !ok || info.mode == UNSELECTED {
+		return nil
+	}
+
+	included := set.NewFromSlice(info.includedLineIndices)
+	indices := []int{}
+	for idx, line := range Parse(info.diff).Lines() {
+		if (line.IsAddition() || line.IsDeletion()) && included.Includes(idx) {
+			indices = append(indices, idx)
+		}
+	}
+	return indices
+}
+
 func (p *PatchBuilder) GetFileIncLineIndices(filename string, previousPath string) ([]int, error) {
 	info, err := p.getFileInfo(filename, previousPath)
 	if err != nil {

pkg/gui/controllers/commit_diff_actions.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/jesseduffield/generics/set"
+	"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/commands/patch"
 	"github.com/jesseduffield/lazygit/pkg/gocui"
@@ -43,9 +44,19 @@ func NewCommitDiffActions(
 	return &CommitDiffActions{c: c, panel: panel, target: target}
 }
 
-// PlainDiff hands out the diff the panel is showing, for the given files — the same
-// diff as in the main view, only without the commit's message and stat above it.
-func (self *CommitDiffActions) PlainDiff(_ types.DiffPaneContext, paths []string) string {
+// PlainDiff hands out the diff the asking pane is showing, for the given files — the
+// commit's diff as in the main view, only without the commit's message and stat above it,
+// or the diff the custom patch is previewed as, whose lines are the patch's own rather
+// than the commit's.
+//
+// The patch's own diff is handed out whole: it is only ever as big as the patch, and it
+// names its files under the trees the patch was materialized into rather than under the
+// paths asked for.
+func (self *CommitDiffActions) PlainDiff(pane types.DiffPaneContext, paths []string) string {
+	if self.showsCustomPatch(pane) {
+		return self.customPatchDiff()
+	}
+
 	target := self.target()
 	if target == nil {
 		return ""
@@ -53,6 +64,22 @@ func (self *CommitDiffActions) PlainDiff(_ types.DiffPaneContext, paths []string
 	return self.c.Helpers().Diff.PlainDiffBetweenRefs(target.from, target.to, paths)
 }
 
+// customPatchDiff is the diff the custom patch is previewed as, as git writes it — the
+// diff behind what the pane previewing the patch shows, in which the lines shown there
+// can be found again.
+func (self *CommitDiffActions) customPatchDiff() string {
+	treesDir := self.c.Git().Patch.PatchBuilder.TempDir()
+	if treesDir == "" {
+		return ""
+	}
+	// An error means the two trees differ, as they do for any patch with something in
+	// it. We are after the diff itself either way.
+	diff, _ := self.c.Git().Diff.
+		CustomPatchDiffCmdObj(treesDir, git_commands.DiffModePlain).
+		RunWithOutput()
+	return diff
+}
+
 // PrimaryAction takes the selected lines into the custom patch being built from this
 // diff, or back out of it when the first of them is already in — the same toggling the
 // commit files panel does to a whole file at a time.
@@ -60,10 +87,10 @@ func (self *CommitDiffActions) PlainDiff(_ types.DiffPaneContext, paths []string
 // The commit is not touched, so the diff stays as it is: what changes is the patch
 // beside it, and which of its lines are marked as being in that patch.
 func (self *CommitDiffActions) PrimaryAction(pane types.DiffPaneContext, firstLineIdx int, lastLineIdx int) error {
-	// Only the diff has lines to take into the patch; the pane beside it shows the patch
-	// they are taken into.
+	// In the pane showing the patch, the lines are the patch's own, so there they only
+	// come back out of it.
 	if self.showsCustomPatch(pane) {
-		return nil
+		return self.removePatchLines(pane, firstLineIdx, lastLineIdx)
 	}
 
 	if self.c.UserConfig().Git.DiffContextSize == 0 {
@@ -125,6 +152,62 @@ func (self *CommitDiffActions) PrimaryAction(pane types.DiffPaneContext, firstLi
 	})
 }
 
+// removePatchLines takes the selected lines of the custom patch out of it. The primary
+// action does this in the pane showing the patch: everything shown there is in the patch
+// already, so there is nothing else it could mean.
+//
+// A line of the patch is named by its position among its file's changes, counted in the
+// diff the patch is shown as. That position is the same one the line has among the
+// changes the patch holds for that file. Line numbers would not do: a patch that leaves
+// an earlier addition out numbers everything after it differently from the commit's diff.
+func (self *CommitDiffActions) removePatchLines(
+	pane types.DiffPaneContext, firstLineIdx int, lastLineIdx int,
+) error {
+	lines := self.c.Helpers().DiffLine.ChangeLinesInViewRange(pane.GetView(), firstLineIdx, lastLineIdx)
+	if len(lines) == 0 {
+		return nil
+	}
+
+	patchBuilder := self.c.Git().Patch.PatchBuilder
+	previousPaths := self.previousPaths()
+	for path, ordinals := range self.c.Helpers().DiffLine.ChangeLineOrdinals(self.customPatchDiff(), lines) {
+		filename := self.patchBuilderPath(path)
+		if filename == "" {
+			continue
+		}
+		included := patchBuilder.IncludedChangeLineIndices(filename)
+		indices := []int{}
+		for _, ordinal := range ordinals {
+			if ordinal < len(included) {
+				indices = append(indices, included[ordinal])
+			}
+		}
+		if len(indices) == 0 {
+			continue
+		}
+		if err := patchBuilder.RemoveFileLineRange(filename, previousPaths[filename], indices); err != nil {
+			return err
+		}
+	}
+	// Taking the last line out ends the patch rather than leaving an empty one, as it does
+	// in the diff beside this pane.
+	if patchBuilder.IsEmpty() {
+		patchBuilder.Reset()
+	}
+
+	self.c.Helpers().DiffLine.RefreshInclusionGutter()
+
+	// The lines are gone from the patch, so the selection carries on from where they were,
+	// as unstaging leaves it. Input is held until it has moved, so that a second press acts
+	// on the patch as it now is.
+	self.c.GocuiGui().BeginBlockingEvents()
+	self.c.Helpers().DiffLine.RevealSelectionAfterAction(pane, pane, firstLineIdx, 0,
+		self.c.GocuiGui().EndBlockingEvents)
+
+	self.c.PostRefreshUpdate(self.panel)
+	return nil
+}
+
 // DiscardSelection takes the selected lines out of the commit they are part of, by
 // building a patch of exactly those lines and removing that patch from the commit. It is
 // a rebase, so a later commit that touches the same lines can conflict with it.

pkg/gui/controllers/helpers/diff_line_helper.go

@@ -66,7 +66,7 @@ func (self *DiffLineHelper) diffLineIdentitiesAt(
 
 	if renderingStatesDiffLines(contents) {
 		if identities := self.diffLineIdentitiesFromRecords(contents[bufferLineIdx].Metadata); len(identities) > 0 {
-			return identities, true
+			return self.inRepoTerms(view, identities), true
 		}
 		return nil, false
 	}
@@ -76,7 +76,7 @@ func (self *DiffLineHelper) diffLineIdentitiesAt(
 		return nil, false
 	}
 
-	return []types.DiffLineInfo{self.diffLineInfo(parsed)}, true
+	return self.inRepoTerms(view, []types.DiffLineInfo{self.diffLineInfo(parsed)}), true
 }
 
 // diffLineIdentitiesFromRecords recovers the identity of every diff line the row's

pkg/gui/controllers/helpers/diff_line_queries.go

@@ -1,6 +1,9 @@
 package helpers
 
 import (
+	"path/filepath"
+	"strings"
+
 	"github.com/jesseduffield/lazygit/pkg/gocui"
 	"github.com/jesseduffield/lazygit/pkg/gui/types"
 	"github.com/samber/lo"
@@ -32,7 +35,81 @@ func (self *DiffLineHelper) DiffLinesInViewRange(view *gocui.View, first int, la
 		previousBufferLine = bufferLine
 		infos = append(infos, identities[bufferLine]...)
 	}
-	return infos
+	return self.inRepoTerms(view, infos)
+}
+
+// ChangeLineOrdinals says, for each of the given change lines, which of its file's
+// changes it is in the given diff — its place among them, counted from the top of the
+// file — keyed by file. Lines the diff doesn't have are left out.
+//
+// It is how a line is named in something built out of a diff rather than being that diff:
+// the custom patch holds the lines it was given in the order the file has them, so a
+// place among a file's changes is a line of the patch.
+func (self *DiffLineHelper) ChangeLineOrdinals(
+	diff string, infos []types.DiffLineInfo,
+) map[string][]int {
+	ordinals := map[patchLine]int{}
+	counts := map[string]int{}
+	for _, parsed := range parseAllDiffLinesFromBuffer(strings.Split(diff, "\n")) {
+		if !parsed.ok {
+			continue
+		}
+		info := self.diffLineInfo(parsed.parsed)
+		if !info.IsChange() {
+			continue
+		}
+		ordinals[patchLineOf(info)] = counts[info.Path]
+		counts[info.Path]++
+	}
+
+	ordinalsByPath := map[string][]int{}
+	for _, info := range infos {
+		if ordinal, ok := ordinals[patchLineOf(info)]; ok {
+			ordinalsByPath[info.Path] = append(ordinalsByPath[info.Path], ordinal)
+		}
+	}
+	return ordinalsByPath
+}
+
+// inRepoTerms brings the paths of lines recovered from a view into the repo's terms.
+//
+// They are in them already for a diff of the repo's own files. The pane previewing the
+// custom patch, though, shows a diff of the two trees the patch was materialized into: a
+// diff renderer states the path it was handed there, which is under the tree's own name,
+// while the diff's text names the trees where an ordinary diff has git's a/ and b/
+// prefixes and so needs nothing.
+func (self *DiffLineHelper) inRepoTerms(view *gocui.View, infos []types.DiffLineInfo) []types.DiffLineInfo {
+	if !self.ShowsCustomPatch(view) {
+		return infos
+	}
+
+	worktreePath := self.c.Git().RepoPaths.WorktreePath()
+	treesDir := self.c.Git().Patch.PatchBuilder.TempDir()
+	return lo.Map(infos, func(info types.DiffLineInfo, _ int) types.DiffLineInfo {
+		info.Path = repoPathOfTreePath(info.Path, treesDir, worktreePath)
+		return info
+	})
+}
+
+// repoPathOfTreePath maps a path under one of the trees the custom patch was materialized
+// into to the file of the repo it stands for: the path is the tree's name followed by the
+// file's own, stated either against the directory holding the trees or against the repo,
+// depending on how the renderer that stated it was given it.
+func repoPathOfTreePath(path string, treesDir string, worktreePath string) string {
+	root := worktreePath
+	if treesDir != "" && strings.HasPrefix(path, treesDir+string(filepath.Separator)) {
+		root = treesDir
+	}
+	relativePath, err := filepath.Rel(root, path)
+	if err != nil {
+		return path
+	}
+
+	segments := strings.Split(filepath.ToSlash(relativePath), "/")
+	if len(segments) > 1 && (segments[0] == "a" || segments[0] == "b") {
+		relativePath = filepath.Join(segments[1:]...)
+	}
+	return filepath.Join(worktreePath, relativePath)
 }
 
 // ChangeLinesInViewRange returns the change lines — the additions and deletions —

pkg/gui/controllers/helpers/diff_line_selection.go

@@ -191,3 +191,10 @@ func (self *DiffLineHelper) patchInclusion() func(types.DiffLineInfo) bool {
 	}
 	return actions.PatchInclusion()
 }
+
+// ShowsCustomPatch reports whether the given view is the one previewing the custom patch
+// being built, which is the lower pane while a patch is being built from the diff in the
+// upper one.
+func (self *DiffLineHelper) ShowsCustomPatch(view *gocui.View) bool {
+	return view == self.c.Contexts().NormalSecondary.GetView() && self.patchInclusion() != nil
+}

pkg/integration/tests/main_view/copy_selected_diff_lines.go

@@ -139,5 +139,25 @@ var CopySelectedDiffLines = NewIntegrationTest(NewIntegrationTestArgs{
 			`\A {4}two\n---\n file1 \|[^\n]*\n 1 file changed[^\n]*\n\n`+
 				`diff --git a/file1 b/file1\nindex [0-9a-f]+\.\.[0-9a-f]+ 100644\n`+
 				`--- a/file1\n\+\+\+ b/file1\n@@ -1,3 \+1,3 @@\n one\n-two\n\z`))
+
+		// The pane showing the custom patch is a diff too, of the patch's own lines.
+		t.Views().Main().
+			Press(keys.Main.ToggleSelectHunk).
+			SelectedLines(
+				Contains("-two <<<"),
+				Contains("+TWO <<<"),
+			).
+			PressPrimaryAction().
+			Press(keys.Universal.TogglePanel)
+
+		t.Views().Secondary().
+			IsFocused().
+			SelectedLines(
+				Contains("-two <<<"),
+				Contains("+TWO <<<"),
+			).
+			Press(keys.Universal.CopyToClipboard)
+
+		expectClipboard(t, Equals("-two\n+TWO\n"))
 	},
 })

pkg/integration/tests/main_view/remove_lines_from_the_custom_patch.go

@@ -0,0 +1,90 @@
+package main_view
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var RemoveLinesFromTheCustomPatch = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Take a line back out of the custom patch from the pane previewing it, where the patch's own numbering is not the commit's",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig: func(config *config.AppConfig) {
+		config.GetUserConfig().Gui.UseHunkModeInStagingView = false
+	},
+	SetupRepo: func(shell *Shell) {
+		shell.CreateFileAndAdd("file1", "one\ntwo\nthree\nfour\n")
+		shell.Commit("first commit")
+
+		// Additions with a line of the file between them, so that leaving the first of
+		// them out of the patch puts the others at line numbers the commit's diff has
+		// unchanged lines at: a line of the patch can only be found by counting the
+		// patch's own changes.
+		shell.UpdateFileAndAdd("file1", "one\nadded a\ntwo\nadded b\nthree\nadded c\nfour\n")
+		shell.Commit("second commit")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			Press(keys.Universal.FocusMainView)
+
+		// Take the second and third additions into the patch, leaving the first out.
+		t.Views().Main().
+			IsFocused().
+			SelectedLines(
+				Contains("+added a"),
+			).
+			NavigateToLine(Contains("+added b")).
+			Press(keys.Universal.ToggleRangeSelect).
+			NavigateToLine(Contains("+added c")).
+			PressPrimaryAction().
+			MarkedLines(
+				Contains("+added b"),
+				Contains("+added c"),
+			)
+
+		t.Views().Secondary().ContainsLines(
+			Contains("+added b"),
+			Contains(" three"),
+			Contains("+added c"),
+		)
+
+		// Point at the first of the patch's lines and take it back out.
+		t.Views().Main().Press(keys.Universal.TogglePanel)
+
+		t.Views().Secondary().
+			IsFocused().
+			SelectedLines(
+				Contains("+added b"),
+			).
+			PressPrimaryAction().
+			// What is left is the line that wasn't pointed at, and the selection has
+			// stayed with it.
+			SelectedLines(
+				Contains("+added c"),
+			).
+			Content(DoesNotContain("+added b"))
+
+		t.Views().Main().MarkedLines(
+			Contains("+added c"),
+		)
+
+		// And the patch really is only that line: applying it to the working tree brings
+		// back nothing else.
+		t.Common().SelectPatchOption(Contains("Apply patch in reverse"))
+
+		t.Views().Files().
+			Focus().
+			Lines(
+				Contains("M").Contains("file1"),
+			)
+
+		// The patch went to the index as well as the working tree, so the file's changes
+		// are on the staged side of its diff.
+		t.Views().Secondary().
+			ContainsLines(
+				Contains("-added c"),
+			).
+			Content(DoesNotContain("+added b"))
+	},
+})

pkg/integration/tests/test_list.go

@@ -415,6 +415,7 @@ var tests = []*components.IntegrationTest{
 	main_view.PatchMarksShowWhileTheDiffIsFocused,
 	main_view.RangeSelectDiffLines,
 	main_view.RawFallbackUnderAnExternalDiff,
+	main_view.RemoveLinesFromTheCustomPatch,
 	main_view.ResetAPatchBuiltFromACommitsDiff,
 	main_view.ResetThePatchFromThePaneShowingIt,
 	main_view.SearchCollapsesTheSelection,