diff --git a/pkg/commands/patch/patch_builder.go b/pkg/commands/patch/patch_builder.go index 8d94bdc9b..fee20c455 100644 --- a/pkg/commands/patch/patch_builder.go +++ b/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 { diff --git a/pkg/gui/controllers/commit_diff_actions.go b/pkg/gui/controllers/commit_diff_actions.go index 79fff258f..b23c0961a 100644 --- a/pkg/gui/controllers/commit_diff_actions.go +++ b/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, firstBufferLine int, lastBufferLine 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, firstBufferLine, lastBufferLine) } if self.c.UserConfig().Git.DiffContextSize == 0 { @@ -125,6 +152,62 @@ func (self *CommitDiffActions) PrimaryAction(pane types.DiffPaneContext, firstBu }) } +// 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, firstBufferLine int, lastBufferLine int, +) error { + lines := self.c.Helpers().DiffLine.ChangeLinesInBufferRange(pane.GetView(), firstBufferLine, lastBufferLine) + 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, firstBufferLine, 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. diff --git a/pkg/gui/controllers/helpers/diff_line_helper.go b/pkg/gui/controllers/helpers/diff_line_helper.go index b26262bba..0da3f858d 100644 --- a/pkg/gui/controllers/helpers/diff_line_helper.go +++ b/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 diff --git a/pkg/gui/controllers/helpers/diff_line_queries.go b/pkg/gui/controllers/helpers/diff_line_queries.go index a7b8874e6..00aedbbf2 100644 --- a/pkg/gui/controllers/helpers/diff_line_queries.go +++ b/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" @@ -31,7 +34,81 @@ func (self *DiffLineHelper) DiffLinesInBufferRange(view *gocui.View, first int, for bufferLine := first; bufferLine <= min(last, len(identities)-1); 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) } // ChangeLinesInBufferRange returns the change lines — the additions and deletions — diff --git a/pkg/gui/controllers/helpers/diff_line_selection.go b/pkg/gui/controllers/helpers/diff_line_selection.go index fd28f0db8..61e9e3046 100644 --- a/pkg/gui/controllers/helpers/diff_line_selection.go +++ b/pkg/gui/controllers/helpers/diff_line_selection.go @@ -194,3 +194,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 +} diff --git a/pkg/integration/tests/main_view/copy_selected_diff_lines.go b/pkg/integration/tests/main_view/copy_selected_diff_lines.go index 26ac538c7..9b5252c7a 100644 --- a/pkg/integration/tests/main_view/copy_selected_diff_lines.go +++ b/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")) }, }) diff --git a/pkg/integration/tests/main_view/remove_lines_from_the_custom_patch.go b/pkg/integration/tests/main_view/remove_lines_from_the_custom_patch.go new file mode 100644 index 000000000..493c287e7 --- /dev/null +++ b/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")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 9d7bfa4a0..b0742eadd 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -417,6 +417,7 @@ var tests = []*components.IntegrationTest{ main_view.PatchMarksShowWheneverTheirDiffIsOnScreen, main_view.RangeSelectDiffLines, main_view.RawFallbackUnderAnExternalDiff, + main_view.RemoveLinesFromTheCustomPatch, main_view.RenderTheDiffBesideThePatchMarks, main_view.ResetAPatchBuiltFromACommitsDiff, main_view.ResetThePatchFromThePaneShowingIt,