diff --git a/pkg/gui/controllers/commit_diff_actions.go b/pkg/gui/controllers/commit_diff_actions.go index bd3ebb7ab..4ee20accc 100644 --- a/pkg/gui/controllers/commit_diff_actions.go +++ b/pkg/gui/controllers/commit_diff_actions.go @@ -1,7 +1,15 @@ package controllers import ( + "fmt" + "path/filepath" + "strings" + + "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/commands/patch" + "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/samber/lo" ) // CommitDiffActions implements what a panel showing a commit's diff offers on that diff @@ -26,7 +34,7 @@ type commitDiffTarget struct { canRebase bool } -var _ types.FocusedMainViewDiffSource = &CommitDiffActions{} +var _ types.FocusedMainViewActions = &CommitDiffActions{} func NewCommitDiffActions( c *ControllerCommon, panel types.Context, target func() *commitDiffTarget, @@ -43,3 +51,265 @@ func (self *CommitDiffActions) PlainDiff(_ types.DiffPaneContext, paths []string } return self.c.Helpers().Diff.PlainDiffBetweenRefs(target.from, target.to, paths) } + +// 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. +// +// 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. + if self.showsCustomPatch(pane) { + return nil + } + + if self.c.UserConfig().Git.DiffContextSize == 0 { + return fmt.Errorf(self.c.Tr.Actions.NotEnoughContextForCustomPatch, + self.c.UserConfig().Keybinding.Universal.IncreaseContextInDiffView) + } + + target := self.target() + if target == nil { + return nil + } + lines := self.c.Helpers().DiffLine.ChangeLinesInBufferRange(pane.GetView(), firstBufferLine, lastBufferLine) + if len(lines) == 0 { + return nil + } + + patchBuilder := self.c.Git().Patch.PatchBuilder + from, reverse := self.patchEndpoints(target) + // A patch is built from one diff, so building from another one means giving up the + // patch there is — which the user is asked about, as entering the patch builder asks. + mustDiscardPatch := patchBuilder.Active() && patchBuilder.NewPatchRequired(from, target.to, reverse) + return self.c.ConfirmIf(mustDiscardPatch, types.ConfirmOpts{ + Title: self.c.Tr.DiscardPatch, + Prompt: self.c.Tr.DiscardPatchConfirm, + HandleConfirm: func() error { + if mustDiscardPatch { + patchBuilder.Reset() + } + if !patchBuilder.Active() { + patchBuilder.Start(from, target.to, reverse, target.canRebase) + } + + if err := self.togglePatchLines(lines); err != nil { + return err + } + // Taking the last line back out ends the patch rather than leaving an empty + // one, so that the pane previewing it and the marks over the diff go with it. + if patchBuilder.IsEmpty() { + patchBuilder.Reset() + } + + // The selection moves on past the lines just toggled, to the next change of + // the diff — which is still there, a toggle leaving the diff as it was, so + // hold input back until it has moved: a second press meanwhile would toggle + // the same lines straight back. + self.c.GocuiGui().BeginBlockingEvents() + self.c.Helpers().DiffLine.RevealSelectionAfterAction(pane, pane, firstBufferLine, len(lines), + self.c.GocuiGui().EndBlockingEvents) + + // The panel's own render, which is all that is needed: the marks over the diff + // and the patch previewed beside it have changed, while the commit has not. + 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. +// +// The patch it needs is its own, so a patch being built is given up first — which the +// prompt says, there being no way to get it back. +func (self *CommitDiffActions) DiscardSelection(pane types.DiffPaneContext, firstBufferLine int, lastBufferLine int) error { + target := self.target() + if target == nil { + return nil + } + lines := self.c.Helpers().DiffLine.ChangeLinesInBufferRange(pane.GetView(), firstBufferLine, lastBufferLine) + if len(lines) == 0 { + return nil + } + commitIndex := self.indexOfTargetCommit(target) + if commitIndex == -1 { + return nil + } + + patchBuilder := self.c.Git().Patch.PatchBuilder + prompt := lo.Ternary(patchBuilder.IsEmpty(), + self.c.Tr.DiscardLinesFromCommitPrompt, + self.c.Tr.DiscardLinesFromCommitPromptWithReset) + + self.c.Confirm(types.ConfirmOpts{ + Title: self.c.Tr.DiscardLinesFromCommitTitle, + Prompt: prompt, + HandleConfirm: func() error { + from, reverse := self.patchEndpoints(target) + patchBuilder.Reset() + patchBuilder.Start(from, target.to, reverse, target.canRebase) + if err := self.togglePatchLines(lines); err != nil { + return err + } + if patchBuilder.IsEmpty() { + return nil + } + + // The rebase runs on a worker, which may not read the model, so the commits + // it rewrites are taken here. + commits := self.c.Model().Commits + return self.c.WithWaitingStatusBlockingInput(types.WaitingStatusOpts{ + Message: self.c.Tr.RebasingStatus, + HideWorkingTreeState: true, + }, func(gocui.Task) error { + self.c.LogAction(self.c.Tr.Actions.RemovePatchFromCommit) + err := self.c.Git().Patch.DeletePatchesFromCommit(commits, commitIndex) + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) + }) + }, + }) + return nil +} + +// DiscardSelectionDisabledReason says why the selected lines can't be taken out of the +// commit: doing so rewrites it, which is only ours to do for a commit of the branch we +// are on, and not while a rebase is already under way. In the pane previewing the custom +// patch there is nothing to discard from — the lines there are the patch's, and space +// takes them back out of it. +func (self *CommitDiffActions) DiscardSelectionDisabledReason(pane types.DiffPaneContext) *types.DisabledReason { + if self.showsCustomPatch(pane) { + return &types.DisabledReason{Text: self.c.Tr.CannotDiscardFromCustomPatchView, ShowErrorInPanel: true} + } + target := self.target() + if target == nil || !target.canRebase { + return &types.DisabledReason{Text: self.c.Tr.CanOnlyDiscardFromLocalCommits, ShowErrorInPanel: true} + } + if self.c.Git().Status.WorkingTreeState().Any() { + return &types.DisabledReason{Text: self.c.Tr.CantPatchWhileRebasingError, ShowErrorInPanel: true} + } + if self.c.UserConfig().Git.DiffContextSize == 0 { + return &types.DisabledReason{ + Text: fmt.Sprintf(self.c.Tr.Actions.NotEnoughContextToRemoveLines, + self.c.UserConfig().Keybinding.Universal.IncreaseContextInDiffView), + ShowErrorInPanel: true, + } + } + return nil +} + +// togglePatchLines takes the given lines of the commit's diff into the custom patch, or +// out of it. The first line of the selection decides which of the two happens, once for +// the whole selection: pointing at a line that is already in the patch takes the whole +// selection out of it, as toggling a selection of files in the commit files panel does. +func (self *CommitDiffActions) togglePatchLines(lines []types.DiffLineInfo) error { + patchBuilder := self.c.Git().Patch.PatchBuilder + + // The files the selection covers, in the order the diff shows them, and per file the + // lines of it that are selected: a patch is built a file at a time, while a selection + // can span several of them. + paths := []string{} + linesByPath := map[string][]patch.LineIdentity{} + for _, line := range lines { + path := self.patchBuilderPath(line.Path) + if path == "" { + continue + } + if _, seen := linesByPath[path]; !seen { + paths = append(paths, path) + } + linesByPath[path] = append(linesByPath[path], line.PatchLineIdentity()) + } + if len(paths) == 0 { + return nil + } + + previousPaths := self.previousPaths() + indicesByPath := map[string][]int{} + for _, path := range paths { + indices, err := patchBuilder.PatchLineIndicesForLines(path, previousPaths[path], linesByPath[path]) + if err != nil { + return err + } + indicesByPath[path] = indices + } + + included, err := patchBuilder.GetFileIncLineIndices(paths[0], previousPaths[paths[0]]) + if err != nil { + return err + } + toggle := patchBuilder.AddFileLineRange + if first := indicesByPath[paths[0]]; len(first) > 0 && lo.Contains(included, first[0]) { + toggle = patchBuilder.RemoveFileLineRange + } + + for _, path := range paths { + if len(indicesByPath[path]) == 0 { + continue + } + if err := toggle(path, previousPaths[path], indicesByPath[path]); err != nil { + return err + } + } + return nil +} + +// previousPaths says which files of the diff were renamed, and what they were called +// before. A renamed file's diff only comes out as a rename when git is asked about both +// of its paths, and its lines are numbered in the file under its old name, so the patch +// builder has to be told the old path along with them. +func (self *CommitDiffActions) previousPaths() map[string]string { + target := self.target() + if target == nil { + return nil + } + from, reverse := self.patchEndpoints(target) + files, err := self.c.Git().Loaders.CommitFileLoader.GetFilesInDiff(from, target.to, reverse) + if err != nil { + return nil + } + + previousPaths := map[string]string{} + for _, file := range files { + if file.PreviousPath != "" { + previousPaths[file.Path] = file.PreviousPath + } + } + return previousPaths +} + +// patchEndpoints gives the two ends of the diff a patch is built from. They are the ends +// of the diff shown, except in diffing mode, where what is shown is a diff against +// another ref, possibly the other way around. +func (self *CommitDiffActions) patchEndpoints(target *commitDiffTarget) (string, bool) { + return self.c.Modes().Diffing.GetFromAndReverseArgsForDiff(target.from) +} + +// patchBuilderPath turns the absolute path a diff line carries into the repo-relative +// one the patch builder keys a file by, and "" for a path that is no file of this repo. +func (self *CommitDiffActions) patchBuilderPath(path string) string { + relativePath, err := filepath.Rel(self.c.Git().RepoPaths.WorktreePath(), path) + if err != nil || strings.HasPrefix(relativePath, "..") { + return "" + } + return filepath.ToSlash(relativePath) +} + +// indexOfTargetCommit finds the commit the diff belongs to among the commits of the +// branch we are on, which is how a rebase is told which commit to rewrite. -1 when it +// isn't one of them, in which case there is nothing we can rewrite. +func (self *CommitDiffActions) indexOfTargetCommit(target *commitDiffTarget) int { + return lo.IndexOf( + lo.Map(self.c.Model().Commits, func(commit *models.Commit, _ int) string { return commit.Hash() }), + target.to) +} + +// showsCustomPatch reports whether the given main pane is the one previewing the custom +// patch being built, rather than the commit's diff — which for a commit's diff is always +// the lower one. +func (self *CommitDiffActions) showsCustomPatch(pane types.DiffPaneContext) bool { + return pane.GetKey() == self.c.Contexts().NormalSecondary.GetKey() +} diff --git a/pkg/gui/controllers/helpers/diff_line_restore.go b/pkg/gui/controllers/helpers/diff_line_restore.go index 3989f4c35..c51bbe31b 100644 --- a/pkg/gui/controllers/helpers/diff_line_restore.go +++ b/pkg/gui/controllers/helpers/diff_line_restore.go @@ -557,11 +557,16 @@ func patchLineOf(info types.DiffLineInfo) patchLine { // in another pane — and shows no selection until the restore places one, so that what // it was left showing the last time it was used doesn't appear for a frame. // +// advanceBy moves on by that many changes past the place remembered, for an action that +// leaves the diff as it was: lines taken into a custom patch are still in the commit's +// diff, so the place remembered is still the line acted on, and carrying on means going +// past the lines just dealt with rather than staying on them. +// // done, which may be nil, is called once the selection is where it belongs, or once it // turns out that no render is coming to put it there — for a caller that must not let // the user act again in between. func (self *DiffLineHelper) RevealSelectionAfterAction( - source types.DiffPaneContext, target types.DiffPaneContext, firstBufferLine int, done func(), + source types.DiffPaneContext, target types.DiffPaneContext, firstBufferLine int, advanceBy int, done func(), ) { ordinal := self.ChangeLineOrdinal(source.GetView(), firstBufferLine) @@ -579,7 +584,7 @@ func (self *DiffLineHelper) RevealSelectionAfterAction( self.c.Context().UpdateSelectionHighlights() } - self.RevealChangeLineAtOrdinal(targetView, ordinal, func(viewLine int) { + self.RevealChangeLineAtOrdinal(targetView, ordinal+advanceBy, func(viewLine int) { if selectHunk { self.SelectChangeBlock(target, viewLine, true) return diff --git a/pkg/gui/controllers/helpers/patch_building_helper.go b/pkg/gui/controllers/helpers/patch_building_helper.go index d8288df4f..bdb106a7b 100644 --- a/pkg/gui/controllers/helpers/patch_building_helper.go +++ b/pkg/gui/controllers/helpers/patch_building_helper.go @@ -43,7 +43,10 @@ func (self *PatchBuildingHelper) Escape() { func (self *PatchBuildingHelper) Reset() error { self.c.Git().Patch.PatchBuilder.Reset() - if self.c.Context().CurrentStatic().GetKind() != types.SIDE_CONTEXT { + // The patch-building view is the one thing with nothing left to show once the patch is + // gone, so it is the one thing left behind. Everywhere else the user is looking at + // something of their own — a commit's diff, the working tree's — which is still there. + if self.c.Context().CurrentStatic().GetKey() == self.c.Contexts().CustomPatchBuilder.GetKey() { self.Escape() } @@ -51,8 +54,11 @@ func (self *PatchBuildingHelper) Reset() error { Scope: []types.RefreshableView{types.COMMIT_FILES}, }) - // refreshing the current context so that the secondary panel is hidden if necessary. - self.c.PostRefreshUpdate(self.c.Context().Current()) + // Render again so that the pane that was previewing the patch goes with it. The + // panel asked to do that is the side panel rather than whichever context has the + // focus. Both main panes are rendered by the panel beneath them, so a reset from + // within the focused main view has to go through that panel too. + self.c.PostRefreshUpdate(self.c.Context().CurrentSide()) return nil } diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 92b0b0e84..251841d5a 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -1086,17 +1086,31 @@ type capturedCommitFilesState struct { from string to string reverse bool + // Whether there is a commit to load the files of at all. The panel is only ever + // pointed at one by being entered, and a patch can now be built from a commit's diff + // without that — after which anything that refreshes the panel would otherwise be + // asking for the files of nothing. + hasCommit bool } // captureCommitFilesState reads the commit-files refresh's diff endpoints into // an immutable snapshot. It must run on the UI thread. func (self *RefreshHelper) captureCommitFilesState() capturedCommitFilesState { - from, to := self.c.Contexts().CommitFiles.GetFromAndToForDiff() + commitFilesContext := self.c.Contexts().CommitFiles + if commitFilesContext.GetRef() == nil && commitFilesContext.GetRefRange() == nil { + return capturedCommitFilesState{} + } + + from, to := commitFilesContext.GetFromAndToForDiff() from, reverse := self.c.Modes().Diffing.GetFromAndReverseArgsForDiff(from) - return capturedCommitFilesState{from: from, to: to, reverse: reverse} + return capturedCommitFilesState{from: from, to: to, reverse: reverse, hasCommit: true} } func (self *RefreshHelper) refreshCommitFilesContext(captured capturedCommitFilesState, env refreshEnv) error { + if !captured.hasCommit { + return nil + } + files, err := env.git.Loaders.CommitFileLoader.GetFilesInDiff(captured.from, captured.to, captured.reverse) if err != nil { return err diff --git a/pkg/gui/controllers/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index a52421ed3..e39578c39 100644 --- a/pkg/gui/controllers/main_view_controller.go +++ b/pkg/gui/controllers/main_view_controller.go @@ -80,22 +80,31 @@ func (self *MainViewController) GetKeybindings(opts types.KeybindingsOpts) []*ty Tooltip: self.c.Tr.EditFileTooltip, }, { - Keys: opts.GetKeys(opts.Config.Universal.Select), - Handler: self.primaryAction, + Keys: opts.GetKeys(opts.Config.Universal.Select), + Handler: self.primaryAction, + // The description is of the working tree's diff, which is where the key does + // the thing users know it for; over a commit's diff it says so for itself. Description: self.c.Tr.Stage, - DescriptionFunc: self.workingTreeActionDescription(self.c.Tr.Stage), + DescriptionFunc: self.diffActionDescription(self.c.Tr.Stage, self.c.Tr.ToggleSelectionForPatch), GetDisabledReason: self.diffSelectionDisabledReason, Tooltip: self.c.Tr.StageSelectionTooltip, - DisplayOnScreen: true, + // Over a commit's diff the key toggles lines in the custom patch, which the + // description says for itself; there is nothing to add to it. + TooltipFunc: self.diffActionDescription(self.c.Tr.StageSelectionTooltip, ""), + DisplayOnScreen: true, }, { Keys: opts.GetKeys(opts.Config.Universal.Remove), Handler: self.discardSelection, Description: self.c.Tr.DiscardSelection, - DescriptionFunc: self.workingTreeActionDescription(self.c.Tr.DiscardSelection), - GetDisabledReason: self.diffSelectionDisabledReason, + DescriptionFunc: self.diffActionDescription(self.c.Tr.DiscardSelection, self.c.Tr.RemoveSelectionFromPatch), + GetDisabledReason: self.discardSelectionDisabledReason, Tooltip: self.c.Tr.DiscardSelectionTooltip, - DisplayOnScreen: true, + // Over a commit's diff the key rewrites the commit rather than touching the + // index, which is worth the warning the other tooltip carries. + TooltipFunc: self.diffActionDescription( + self.c.Tr.DiscardSelectionTooltip, self.c.Tr.RemoveSelectionFromPatchTooltip), + DisplayOnScreen: true, }, { Keys: opts.GetKeys(opts.Config.Main.EditSelectHunk), @@ -390,11 +399,23 @@ func (self *MainViewController) workingTreeAction(action func() error) func() er // workingTreeActionDescription gives a command's description only where the command // applies — over the working tree's diff — so that it is listed there and nowhere else. func (self *MainViewController) workingTreeActionDescription(description string) func() string { + return self.diffActionDescription(description, "") +} + +// diffActionDescription describes a command in the words that suit the diff it applies +// to: acting on the working tree's diff stages, acting on a commit's builds a custom +// patch. Over content that is no diff at all the command doesn't apply, and describes +// itself as nothing, which keeps it out of the keybindings menu there. +func (self *MainViewController) diffActionDescription(staging string, patchBuilding string) func() string { return func() string { - if self.diffMainViewType() != types.DiffMainViewTypeStaging { + switch self.diffMainViewType() { + case types.DiffMainViewTypeStaging: + return staging + case types.DiffMainViewTypePatchBuilding: + return patchBuilding + default: return "" } - return description } } @@ -479,6 +500,19 @@ func (self *MainViewController) diffSelectionDisabledReason() *types.DisabledRea return nil } +// discardSelectionDisabledReason disables discarding while there is nothing to discard, +// and where the panel beneath won't have it: taking lines out of a commit means +// rewriting it, which isn't always something we may do. +func (self *MainViewController) discardSelectionDisabledReason() *types.DisabledReason { + if reason := self.diffSelectionDisabledReason(); reason != nil { + return reason + } + if actions := self.focusedMainViewActions(); actions != nil { + return actions.DiscardSelectionDisabledReason(self.context) + } + return nil +} + func (self *MainViewController) onClickInAlreadyFocusedView(opts gocui.ViewMouseBindingOpts) error { self.selectClickedDiffLine(opts.Y) return nil diff --git a/pkg/gui/controllers/options_menu_action.go b/pkg/gui/controllers/options_menu_action.go index be2899632..2a2b786a3 100644 --- a/pkg/gui/controllers/options_menu_action.go +++ b/pkg/gui/controllers/options_menu_action.go @@ -27,7 +27,7 @@ func (self *OptionsMenuAction) Call() error { if binding.GetDisabledReason != nil { disabledReason = binding.GetDisabledReason() } - tooltip := binding.Tooltip + tooltip := binding.GetTooltip() if len(binding.Keys) > 1 { if tooltip != "" { tooltip += "\n\n" diff --git a/pkg/gui/controllers/reflog_commits_controller.go b/pkg/gui/controllers/reflog_commits_controller.go index 9cfb34f11..b5dac1cdd 100644 --- a/pkg/gui/controllers/reflog_commits_controller.go +++ b/pkg/gui/controllers/reflog_commits_controller.go @@ -77,6 +77,7 @@ func (self *ReflogCommitsController) GetOnRenderToMain() func() { Title: "Reflog Entry", Task: task, }, + Secondary: secondaryPatchPanelUpdateOpts(self.c), }) }) } diff --git a/pkg/gui/controllers/stash_controller.go b/pkg/gui/controllers/stash_controller.go index 7c0b61c66..4303d8851 100644 --- a/pkg/gui/controllers/stash_controller.go +++ b/pkg/gui/controllers/stash_controller.go @@ -108,6 +108,7 @@ func (self *StashController) GetOnRenderToMain() func() { SubTitle: self.c.Helpers().Diff.IgnoringWhitespaceSubTitle(), Task: task, }, + Secondary: secondaryPatchPanelUpdateOpts(self.c), }) }) } diff --git a/pkg/gui/controllers/sub_commits_controller.go b/pkg/gui/controllers/sub_commits_controller.go index d3d0c0b98..7f7a163c9 100644 --- a/pkg/gui/controllers/sub_commits_controller.go +++ b/pkg/gui/controllers/sub_commits_controller.go @@ -56,6 +56,7 @@ func (self *SubCommitsController) GetOnRenderToMain() func() { SubTitle: self.c.Helpers().Diff.IgnoringWhitespaceSubTitle(), Task: task, }, + Secondary: secondaryPatchPanelUpdateOpts(self.c), }) }) } diff --git a/pkg/gui/controllers/working_tree_diff_actions.go b/pkg/gui/controllers/working_tree_diff_actions.go index ec430b3db..e32cdf70f 100644 --- a/pkg/gui/controllers/working_tree_diff_actions.go +++ b/pkg/gui/controllers/working_tree_diff_actions.go @@ -100,6 +100,12 @@ func (self *WorkingTreeDiffActions) DiscardSelection(pane types.DiffPaneContext, }) } +// DiscardSelectionDisabledReason is nil: a change of the working tree can always be +// thrown away, and one in the index always taken back out of it. +func (self *WorkingTreeDiffActions) DiscardSelectionDisabledReason(types.DiffPaneContext) *types.DisabledReason { + return nil +} + // EditHunk opens the git hunk holding the selection in an editor, as a patch against // the index, and applies whatever comes back. It is how you stage something the diff // can't express — half of a changed line, or a change written differently from either @@ -346,7 +352,7 @@ func (self *WorkingTreeDiffActions) revealSelectionInPaneItLandsIn( // that, and until it has been the selection is still on lines that aren't there any // more — so a key pressed meanwhile would act on nothing. self.c.GocuiGui().BeginBlockingEvents() - self.c.Helpers().DiffLine.RevealSelectionAfterAction(pane, target, firstBufferLine, + self.c.Helpers().DiffLine.RevealSelectionAfterAction(pane, target, firstBufferLine, 0, self.c.GocuiGui().EndBlockingEvents) } diff --git a/pkg/gui/main_panels.go b/pkg/gui/main_panels.go index 4fd86d475..c1dabb3b1 100644 --- a/pkg/gui/main_panels.go +++ b/pkg/gui/main_panels.go @@ -297,7 +297,7 @@ func (gui *Gui) keepDiffSelectionAcrossACommitRewrite(opts types.RefreshMainOpts if !ok { continue } - gui.helpers.DiffLine.RevealSelectionAfterAction(mainContext, mainContext, first, nil) + gui.helpers.DiffLine.RevealSelectionAfterAction(mainContext, mainContext, first, 0, nil) } } diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index 4950dfc9d..b32ee8bae 100644 --- a/pkg/gui/types/context.go +++ b/pkg/gui/types/context.go @@ -257,8 +257,14 @@ type FocusedMainViewActions interface { PrimaryAction(pane DiffPaneContext, firstBufferLine int, lastBufferLine int) error // DiscardSelection takes the selected diff lines back out of whatever they are part - // of: the working tree for the files panel. + // of: the working tree for the files panel, the commit itself for the panels showing + // a commit's diff. DiscardSelection(pane DiffPaneContext, firstBufferLine int, lastBufferLine int) error + + // DiscardSelectionDisabledReason says why the selection can't be discarded where it + // is, and nil when it can. Taking lines out of a commit means rewriting it, which + // isn't always something we may do; the working tree has no such condition. + DiscardSelectionDisabledReason(pane DiffPaneContext) *DisabledReason } type IListContext interface { diff --git a/pkg/gui/types/keybindings.go b/pkg/gui/types/keybindings.go index 337162d76..32d0e7fa4 100644 --- a/pkg/gui/types/keybindings.go +++ b/pkg/gui/types/keybindings.go @@ -39,6 +39,11 @@ type Binding struct { // to be displayed if the keybinding is highlighted from within a menu Tooltip string + // TooltipFunc is used instead of Tooltip if non-nil, for a command whose tooltip + // depends on context, as DescriptionFunc is for its description — and with the + // same two conditions: it must not be an expensive call, and a generic Tooltip + // must still be given, since that is the one the cheatsheet prints. + TooltipFunc func() string // Function to decide whether the command is enabled, and why. If this // returns an empty string, it is; if it returns a non-empty string, it is @@ -59,6 +64,13 @@ func (b *Binding) GetDescription() string { return b.Description } +func (b *Binding) GetTooltip() string { + if b.TooltipFunc != nil { + return b.TooltipFunc() + } + return b.Tooltip +} + func (b *Binding) GetShortDescription() string { if b.ShortDescriptionFunc != nil { return b.ShortDescriptionFunc() diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 004ff4288..f9dd54862 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -472,6 +472,7 @@ type TranslationSet struct { CheckoutCommitFileTooltip string CannotCheckoutWithModifiedFilesErr string CanOnlyDiscardFromLocalCommits string + CannotDiscardFromCustomPatchView string CannotDiscardFromMultipleCommits string Remove string DiscardOldFileChangeTooltip string @@ -1665,6 +1666,7 @@ func EnglishTranslationSet() *TranslationSet { CheckoutCommitFileTooltip: "Checkout file. This replaces the file in your working tree with the version from the selected commit.", CannotCheckoutWithModifiedFilesErr: "You have local modifications for the file(s) you are trying to check out. You need to stash or discard these first.", CanOnlyDiscardFromLocalCommits: "Changes can only be discarded from local commits", + CannotDiscardFromCustomPatchView: "Lines shown here are the custom patch's; press space to take them back out of it", CannotDiscardFromMultipleCommits: "Changes cannot be discarded from a multiselection of commits", Remove: "Remove", DiscardOldFileChangeTooltip: "Discard this commit's changes to this file. This runs an interactive rebase in the background, so you may get a merge conflict if a later commit also changes this file.", diff --git a/pkg/integration/tests/main_view/build_patch_from_a_commits_diff.go b/pkg/integration/tests/main_view/build_patch_from_a_commits_diff.go new file mode 100644 index 000000000..b60ff404c --- /dev/null +++ b/pkg/integration/tests/main_view/build_patch_from_a_commits_diff.go @@ -0,0 +1,92 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var BuildPatchFromACommitsDiff = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Take lines of a commit's diff into a custom patch, and back out of it, from the focused main view", + 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\nfive\n") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "ONE\ntwo\nTHREE\nfour\nfive\n") + shell.Commit("second commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("second commit").IsSelected(), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("file1").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("-one"), + ). + // The line goes into the patch, which the pane beside the diff previews, and + // the selection moves on to the next change rather than staying where a + // second press would take it straight back out. + PressPrimaryAction(). + SelectedLines( + Contains("+ONE"), + ) + + t.Views().Information().Content(Contains("Building patch")) + t.Views().Secondary().ContainsLines( + Contains("-one"), + Contains(" two"), + ) + + // The addition of the same modification goes in too, and the patch holds both. + t.Views().Main(). + IsFocused(). + PressPrimaryAction(). + SelectedLines( + Contains("-three"), + ) + + t.Views().Secondary().ContainsLines( + Contains("-one"), + Contains("+ONE"), + Contains(" two"), + ) + + // Pointing at a line that is in the patch takes it back out. + t.Views().Main(). + IsFocused(). + NavigateToLine(Contains("+ONE")). + PressPrimaryAction() + + t.Views().Secondary(). + ContainsLines( + Contains("-one"), + Contains(" two"), + ). + Content(DoesNotContain("+ONE")) + + // Taking the last line out ends the patch, so the pane previewing it goes away. + t.Views().Main(). + IsFocused(). + NavigateToLine(Contains("-one")). + PressPrimaryAction() + + t.Views().Information().Content(DoesNotContain("Building patch")) + }, +}) diff --git a/pkg/integration/tests/main_view/build_patch_from_a_reflog_entry.go b/pkg/integration/tests/main_view/build_patch_from_a_reflog_entry.go new file mode 100644 index 000000000..9531b73d8 --- /dev/null +++ b/pkg/integration/tests/main_view/build_patch_from_a_reflog_entry.go @@ -0,0 +1,54 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var BuildPatchFromAReflogEntry = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Take lines of the diff of a reflog entry into a custom patch, which can then be applied", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInStagingView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\n") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "one\nTWO\nthree\n") + shell.Commit("second commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().ReflogCommits(). + Focus(). + Lines( + Contains("second commit").IsSelected(), + Contains("first commit"), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("-two"), + ). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Views().Secondary().ContainsLines( + Contains("-two"), + Contains(" three"), + ) + + // A reflog entry is never a commit we may rewrite, so the patch can be applied + // but not moved out of the commit it came from. + t.Common().SelectPatchOption(Contains("Apply patch in reverse")) + + t.Views().Files(). + Focus(). + Lines( + Contains("M").Contains("file1"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/build_patch_from_a_whole_commits_diff.go b/pkg/integration/tests/main_view/build_patch_from_a_whole_commits_diff.go new file mode 100644 index 000000000..6d05d6a9d --- /dev/null +++ b/pkg/integration/tests/main_view/build_patch_from_a_whole_commits_diff.go @@ -0,0 +1,75 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var BuildPatchFromAWholeCommitsDiff = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Take lines of two files into a custom patch from the whole diff of a commit, without entering its files first", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInStagingView = true + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\n") + shell.CreateFileAndAdd("file2", "alpha\nbeta\ngamma\n") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "one\nTWO\nthree\n") + shell.UpdateFileAndAdd("file2", "alpha\nBETA\ngamma\n") + shell.Commit("second commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("second commit").IsSelected(), + Contains("first commit"), + ). + Press(keys.Universal.FocusMainView) + + // The commit's whole diff spans both files, and hunk mode offers the first + // changed block of the first of them. + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("-two"), + Contains("+TWO"), + ). + PressPrimaryAction(). + // The selection moves past the block just taken in, which is file2's block. + SelectedLines( + Contains("-beta"), + Contains("+BETA"), + ). + PressPrimaryAction() + + // The patch spans both files. + t.Views().Secondary(). + Content(Contains("file1")). + Content(Contains("file2")). + ContainsLines( + Contains("-two"), + Contains("+TWO"), + Contains(" three"), + ). + ContainsLines( + Contains("-beta"), + Contains("+BETA"), + Contains(" gamma"), + ) + + // Applying it to the working tree puts both files' changes there, which is the + // proof that the patch really holds what the preview says it does. + t.Common().SelectPatchOption(Contains("Apply patch in reverse")) + + t.Views().Files(). + Focus(). + ContainsLines( + Contains("M").Contains("file1"), + Contains("M").Contains("file2"), + ) + }, +}) diff --git a/pkg/integration/tests/main_view/discard_from_a_commit_only_where_it_can_be_rewritten.go b/pkg/integration/tests/main_view/discard_from_a_commit_only_where_it_can_be_rewritten.go new file mode 100644 index 000000000..1458bf8ba --- /dev/null +++ b/pkg/integration/tests/main_view/discard_from_a_commit_only_where_it_can_be_rewritten.go @@ -0,0 +1,63 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var DiscardFromACommitOnlyWhereItCanBeRewritten = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Discarding lines is refused over a diff that belongs to no commit we may rewrite, and over the custom patch itself", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInStagingView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\n") + shell.Commit("first commit") + + shell.UpdateFile("file1", "one\nTWO\nthree\n") + shell.Stash("a stashed change") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + // A stash entry is no commit of ours to rewrite, so its lines can go into a + // patch but can't be taken out of what they are part of. + t.Views().Stash(). + Focus(). + Lines( + Contains("a stashed change").IsSelected(), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("-two"), + ). + Press(keys.Universal.Remove) + + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("Changes can only be discarded from local commits")). + Confirm() + + // The pane previewing the patch shows the patch's own lines, which are not the + // commit's to discard; space takes them back out of the patch instead. + t.Views().Main(). + IsFocused(). + PressPrimaryAction(). + Press(keys.Universal.TogglePanel) + + t.Views().Secondary(). + IsFocused(). + SelectedLines( + Contains("-two"), + ). + Press(keys.Universal.Remove) + + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content(Contains("Lines shown here are the custom patch's")). + Confirm() + }, +}) diff --git a/pkg/integration/tests/main_view/discard_lines_from_a_commit.go b/pkg/integration/tests/main_view/discard_lines_from_a_commit.go new file mode 100644 index 000000000..393e43381 --- /dev/null +++ b/pkg/integration/tests/main_view/discard_lines_from_a_commit.go @@ -0,0 +1,62 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var DiscardLinesFromACommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Discard the selected lines of a commit's diff from the commit itself, in the focused main view", + 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\nfive\n") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "one\nTWO\nthree\nFOUR\nfive\n") + shell.Commit("second commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("second commit").IsSelected(), + Contains("first commit"), + ). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("-two"), + ). + Press(keys.Universal.ToggleRangeSelect). + NavigateToLine(Contains("+TWO")). + SelectedLines( + Contains("-two"), + Contains("+TWO"), + ). + Press(keys.Universal.Remove) + + t.ExpectPopup().Confirmation(). + Title(Equals("Discard lines from commit")). + Content(Contains("Are you sure you want to discard the selected lines from this commit?")). + Confirm() + + // The commit keeps its other change and has given up the one discarded, and the + // selection carries on from the change that has taken its place. + t.Views().Main(). + IsFocused(). + Content(DoesNotContain("+TWO")). + SelectedLines( + Contains("-four"), + ) + + // The rewrite is the commit's own business: nothing is left lying in the working + // tree. + t.Views().Files().IsEmpty() + }, +}) diff --git a/pkg/integration/tests/main_view/reset_a_patch_built_from_a_commits_diff.go b/pkg/integration/tests/main_view/reset_a_patch_built_from_a_commits_diff.go new file mode 100644 index 000000000..7a65d90ff --- /dev/null +++ b/pkg/integration/tests/main_view/reset_a_patch_built_from_a_commits_diff.go @@ -0,0 +1,49 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ResetAPatchBuiltFromACommitsDiff = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Reset a custom patch built from a commit's diff without ever entering the commit's files, and stay in the diff", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.UseHunkModeInStagingView = false + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\n") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "one\nTWO\nthree\n") + shell.Commit("second commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + // Build the patch straight from the commit's diff, so that nothing has ever told + // the commit files panel which commit it would be showing. + t.Views().Commits(). + Focus(). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("-two"), + ). + PressPrimaryAction() + + t.Views().Information().Content(Contains("Building patch")) + t.Views().Secondary().IsVisible().Content(Contains("-two")) + + t.Common().SelectPatchOption(Contains("Reset patch")) + + // Giving up the patch leaves the diff it was being built from, and the focus in it, + // while the pane that was previewing the patch goes with it. + t.Views().Information().Content(DoesNotContain("Building patch")) + t.Views().Secondary().IsInvisible() + t.Views().Main(). + IsFocused(). + Content(Contains("-two")) + }, +}) diff --git a/pkg/integration/tests/main_view/reset_the_patch_from_the_pane_showing_it.go b/pkg/integration/tests/main_view/reset_the_patch_from_the_pane_showing_it.go new file mode 100644 index 000000000..0cebe5d8c --- /dev/null +++ b/pkg/integration/tests/main_view/reset_the_patch_from_the_pane_showing_it.go @@ -0,0 +1,49 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ResetThePatchFromThePaneShowingIt = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Reset a custom patch while the focus is in the pane previewing it", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\n") + shell.Commit("first commit") + + shell.UpdateFileAndAdd("file1", "one\nTWO\nthree\n") + shell.Commit("second commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + SelectedLines( + Contains("-two"), + Contains("+TWO"), + ). + PressPrimaryAction(). + Press(keys.Universal.TogglePanel) + + t.Views().Information().Content(Contains("Building patch")) + t.Views().Secondary(). + IsFocused(). + Content(Contains("-two")) + + t.Common().SelectPatchOption(Contains("Reset patch")) + + // The pane goes with the patch it was previewing, and the focus follows into the + // one showing the diff the patch was built from. + t.Views().Information().Content(DoesNotContain("Building patch")) + t.Views().Secondary().IsInvisible() + t.Views().Main(). + IsFocused(). + Content(Contains("-two")) + }, +}) diff --git a/pkg/integration/tests/main_view/selection_command_tooltips_follow_the_diff.go b/pkg/integration/tests/main_view/selection_command_tooltips_follow_the_diff.go new file mode 100644 index 000000000..d49b3cdec --- /dev/null +++ b/pkg/integration/tests/main_view/selection_command_tooltips_follow_the_diff.go @@ -0,0 +1,62 @@ +package main_view + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var SelectionCommandTooltipsFollowTheDiff = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "The tooltips of the selection commands describe what they do over the diff they are offered on", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\nthree\n") + shell.Commit("one") + + shell.UpdateFile("file1", "one\nTWO\nthree\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + // Over the working tree's diff the two keys act on the index. + t.Views().Files(). + Focus(). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + Press(keys.Universal.OptionMenu) + + t.ExpectPopup().Menu(). + Title(Equals("Keybindings")). + Select(Contains("Stage")). + Tooltip(Equals("Toggle selection staged / unstaged.")). + Select(Contains("Discard")). + Tooltip(Contains("discard the change using `git reset`")). + Cancel() + + t.Views().Main().PressEscape() + + // Over a commit's diff they build a custom patch and rewrite the commit, so + // the index wording would be wrong; taking lines out of a commit is worth a + // warning of its own. + t.Views().Commits(). + Focus(). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + Press(keys.Universal.OptionMenu) + + t.ExpectPopup().Menu(). + Title(Equals("Keybindings")). + Select(Contains("Toggle lines in patch")). + Tooltip(Equals("")). + Select(Contains("Remove lines from commit")). + Tooltip(Contains("runs an interactive rebase in the background")). + Cancel() + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index e581e8efd..499ee9c23 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -368,11 +368,16 @@ var tests = []*components.IntegrationTest{ interactive_rebase.SwapWithConflict, interactive_rebase.ViewFilesOfTodoEntries, main_view.AdvanceAfterStagingShiftsLineNumbers, + main_view.BuildPatchFromACommitsDiff, + main_view.BuildPatchFromAReflogEntry, + main_view.BuildPatchFromAWholeCommitsDiff, main_view.ClickSelectsDiffLine, main_view.CommitFromMainView, main_view.CopyRowsThatAreNoDiffLine, main_view.CopySelectedDiffLines, main_view.DiscardDiffLines, + main_view.DiscardFromACommitOnlyWhereItCanBeRewritten, + main_view.DiscardLinesFromACommit, main_view.DragRangeWithAutoscroll, main_view.DragSelectsDiffLineRange, main_view.EditHunkInFocusedDiff, @@ -407,6 +412,8 @@ var tests = []*components.IntegrationTest{ main_view.NoSelectionWhenNoChanges, main_view.RangeSelectDiffLines, main_view.RawFallbackUnderAnExternalDiff, + main_view.ResetAPatchBuiltFromACommitsDiff, + main_view.ResetThePatchFromThePaneShowingIt, main_view.SearchCollapsesTheSelection, main_view.SearchFollowsTheSelection, main_view.SelectBelowALongCommitMessage, @@ -422,6 +429,7 @@ var tests = []*components.IntegrationTest{ main_view.SelectNextDeletionAfterStagingOne, main_view.SelectVisibleChangeOnFocusingMainView, main_view.SelectVisibleHunkOnFocusingMainView, + main_view.SelectionCommandTooltipsFollowTheDiff, main_view.SelectionCommandsOnlyWhereTheyApply, main_view.SelectionOverTheCustomPatch, main_view.StageDeletedFile,