Auto-stash modified files when cherry-picking or reverting commits (#4683)

- **PR Description**

For cherry-picking, this used to work in earlier versions, but it broke
in #4443. For reverting, it was never supported.

Also, we add some minor improvements while we're at it, such as slightly
better names for the auto-stashes that are created for the various
operations (so that, if an auto-stash pop fails and the stash is kept
around, you can tell more easily what it was for). Also, we now adjust
the selection of the commits list after cherry-picking, so that the same
commit stays selected.

.golangci.yml

@@ -99,9 +99,7 @@ linters:
       - legacy
       - std-error-handling
     paths:
-      - third_party$
+      - vendor/
-      - builtin$
-      - examples$
 formatters:
   enable:
     - gofumpt
@@ -109,6 +107,4 @@ formatters:
   exclusions:
     generated: lax
     paths:
-      - third_party$
+      - vendor/
-      - builtin$
-      - examples$

pkg/commands/git_commands/patch.go

@@ -1,6 +1,7 @@
 package git_commands
 
 import (
+	"fmt"
 	"path/filepath"
 	"time"
 
@@ -218,7 +219,7 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s
 
 func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitIdx int, stash bool) error {
 	if stash {
-		if err := self.stash.Push(self.Tr.StashPrefix + commits[commitIdx].Hash()); err != nil {
+		if err := self.stash.Push(fmt.Sprintf(self.Tr.AutoStashForMovingPatchToIndex, commits[commitIdx].ShortHash())); err != nil {
 			return err
 		}
 	}

pkg/gui/controllers/helpers/cherry_pick_helper.go

@@ -3,7 +3,6 @@ package helpers
 import (
 	"strconv"
 
-	"github.com/jesseduffield/gocui"
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/gui/modes/cherrypicking"
 	"github.com/jesseduffield/lazygit/pkg/gui/types"
@@ -77,14 +76,34 @@ func (self *CherryPickHelper) Paste() error {
 				"numCommits": strconv.Itoa(len(self.getData().CherryPickedCommits)),
 			}),
 		HandleConfirm: func() error {
-			return self.c.WithWaitingStatus(self.c.Tr.CherryPickingStatus, func(gocui.Task) error {
+			return self.c.WithWaitingStatusSync(self.c.Tr.CherryPickingStatus, func() error {
+				mustStash := IsWorkingTreeDirty(self.c.Model().Files)
+
 				self.c.LogAction(self.c.Tr.Actions.CherryPick)
-				result := self.c.Git().Rebase.CherryPickCommits(self.getData().CherryPickedCommits)
+
-				err := self.rebaseHelper.CheckMergeOrRebase(result)
+				if mustStash {
+					if err := self.c.Git().Stash.Push(self.c.Tr.AutoStashForCherryPicking); err != nil {
+						return err
+					}
+				}
+
+				cherryPickedCommits := self.getData().CherryPickedCommits
+				result := self.c.Git().Rebase.CherryPickCommits(cherryPickedCommits)
+				err := self.rebaseHelper.CheckMergeOrRebaseWithRefreshOptions(result, types.RefreshOptions{Mode: types.SYNC})
 				if err != nil {
 					return result
 				}
 
+				// Move the selection down by the number of commits we just
+				// cherry-picked, to keep the same commit selected as before.
+				// Don't do this if a rebase todo is selected, because in this
+				// case we are in a rebase and the cherry-picked commits end up
+				// below the selection.
+				if commit := self.c.Contexts().LocalCommits.GetSelected(); commit != nil && !commit.IsTODO() {
+					self.c.Contexts().LocalCommits.MoveSelection(len(cherryPickedCommits))
+					self.c.Contexts().LocalCommits.FocusLine()
+				}
+
 				// If we're in the cherry-picking state at this point, it must
 				// be because there were conflicts. Don't clear the copied
 				// commits in this case, since we might want to abort and try
@@ -96,7 +115,17 @@ func (self *CherryPickHelper) Paste() error {
 				if !isInCherryPick {
 					self.getData().DidPaste = true
 					self.rerender()
+
+					if mustStash {
+						if err := self.c.Git().Stash.Pop(0); err != nil {
+							return err
+						}
+						self.c.Refresh(types.RefreshOptions{
+							Scope: []types.RefreshableView{types.STASH, types.FILES},
+						})
+					}
 				}
+
 				return nil
 			})
 		},

pkg/gui/controllers/helpers/refs_helper.go

@@ -79,7 +79,7 @@ func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions
 						Prompt: self.c.Tr.AutoStashPrompt,
 						HandleConfirm: func() error {
 							return withCheckoutStatus(func(gocui.Task) error {
-								if err := self.c.Git().Stash.Push(self.c.Tr.StashPrefix + ref); err != nil {
+								if err := self.c.Git().Stash.Push(fmt.Sprintf(self.c.Tr.AutoStashForCheckout, ref)); err != nil {
 									return err
 								}
 								if err := self.c.Git().Branch.Checkout(ref, cmdOptions); err != nil {
@@ -355,7 +355,7 @@ func (self *RefsHelper) NewBranch(from string, fromFormattedName string, suggest
 						Title:  self.c.Tr.AutoStashTitle,
 						Prompt: self.c.Tr.AutoStashPrompt,
 						HandleConfirm: func() error {
-							if err := self.c.Git().Stash.Push(self.c.Tr.StashPrefix + newBranchName); err != nil {
+							if err := self.c.Git().Stash.Push(fmt.Sprintf(self.c.Tr.AutoStashForNewBranch, newBranchName)); err != nil {
 								return err
 							}
 							if err := newBranchFunc(newBranchName, from); err != nil {
@@ -389,7 +389,7 @@ func (self *RefsHelper) MoveCommitsToNewBranch() error {
 		return err
 	}
 
-	withNewBranchNamePrompt := func(baseBranchName string, f func(string, string) error) error {
+	withNewBranchNamePrompt := func(baseBranchName string, f func(string) error) error {
 		prompt := utils.ResolvePlaceholderString(
 			self.c.Tr.NewBranchNameBranchOff,
 			map[string]string{
@@ -408,7 +408,7 @@ func (self *RefsHelper) MoveCommitsToNewBranch() error {
 				self.c.LogAction(self.c.Tr.MoveCommitsToNewBranch)
 				newBranchName := SanitizedBranchName(response)
 				return self.c.WithWaitingStatus(self.c.Tr.MovingCommitsToNewBranchStatus, func(gocui.Task) error {
-					return f(currentBranch.Name, newBranchName)
+					return f(newBranchName)
 				})
 			},
 		})
@@ -447,8 +447,8 @@ func (self *RefsHelper) MoveCommitsToNewBranch() error {
 			{
 				Label: fmt.Sprintf(self.c.Tr.MoveCommitsToNewBranchFromBaseItem, shortBaseBranchName),
 				OnPress: func() error {
-					return withNewBranchNamePrompt(shortBaseBranchName, func(currentBranch string, newBranchName string) error {
+					return withNewBranchNamePrompt(shortBaseBranchName, func(newBranchName string) error {
-						return self.moveCommitsToNewBranchOffOfMainBranch(currentBranch, newBranchName, baseBranchRef)
+						return self.moveCommitsToNewBranchOffOfMainBranch(newBranchName, baseBranchRef)
 					})
 				},
 			},
@@ -462,14 +462,14 @@ func (self *RefsHelper) MoveCommitsToNewBranch() error {
 	})
 }
 
-func (self *RefsHelper) moveCommitsToNewBranchStackedOnCurrentBranch(currentBranch string, newBranchName string) error {
+func (self *RefsHelper) moveCommitsToNewBranchStackedOnCurrentBranch(newBranchName string) error {
 	if err := self.c.Git().Branch.NewWithoutCheckout(newBranchName, "HEAD"); err != nil {
 		return err
 	}
 
 	mustStash := IsWorkingTreeDirty(self.c.Model().Files)
 	if mustStash {
-		if err := self.c.Git().Stash.Push(self.c.Tr.StashPrefix + currentBranch); err != nil {
+		if err := self.c.Git().Stash.Push(fmt.Sprintf(self.c.Tr.AutoStashForNewBranch, newBranchName)); err != nil {
 			return err
 		}
 	}
@@ -495,14 +495,14 @@ func (self *RefsHelper) moveCommitsToNewBranchStackedOnCurrentBranch(currentBran
 	return nil
 }
 
-func (self *RefsHelper) moveCommitsToNewBranchOffOfMainBranch(currentBranch string, newBranchName string, baseBranchRef string) error {
+func (self *RefsHelper) moveCommitsToNewBranchOffOfMainBranch(newBranchName string, baseBranchRef string) error {
 	commitsToCherryPick := lo.Filter(self.c.Model().Commits, func(commit *models.Commit, _ int) bool {
 		return commit.Status == models.StatusUnpushed
 	})
 
 	mustStash := IsWorkingTreeDirty(self.c.Model().Files)
 	if mustStash {
-		if err := self.c.Git().Stash.Push(self.c.Tr.StashPrefix + currentBranch); err != nil {
+		if err := self.c.Git().Stash.Push(fmt.Sprintf(self.c.Tr.AutoStashForNewBranch, newBranchName)); err != nil {
 			return err
 		}
 	}

pkg/gui/controllers/local_commits_controller.go

@@ -873,14 +873,30 @@ func (self *LocalCommitsController) revert(commits []*models.Commit, start, end
 		HandleConfirm: func() error {
 			self.c.LogAction(self.c.Tr.Actions.RevertCommit)
 			return self.c.WithWaitingStatusSync(self.c.Tr.RevertingStatus, func() error {
+				mustStash := helpers.IsWorkingTreeDirty(self.c.Model().Files)
+
+				if mustStash {
+					if err := self.c.Git().Stash.Push(self.c.Tr.AutoStashForReverting); err != nil {
+						return err
+					}
+				}
+
 				result := self.c.Git().Commit.Revert(hashes, isMerge)
-				if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(result); err != nil {
+				if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions(result, types.RefreshOptions{Mode: types.SYNC}); err != nil {
 					return err
 				}
 				self.context().MoveSelection(len(commits))
-				self.c.Refresh(types.RefreshOptions{
+				self.context().FocusLine()
-					Mode: types.SYNC, Scope: []types.RefreshableView{types.COMMITS, types.BRANCHES},
+
-				})
+				if mustStash {
+					if err := self.c.Git().Stash.Pop(0); err != nil {
+						return err
+					}
+					self.c.Refresh(types.RefreshOptions{
+						Scope: []types.RefreshableView{types.STASH, types.FILES},
+					})
+				}
+
 				return nil
 			})
 		},

pkg/gui/controllers/undo_controller.go

@@ -90,7 +90,7 @@ func (self *UndoController) reflogUndo() error {
 		case COMMIT:
 			self.c.Confirm(types.ConfirmOpts{
 				Title:  self.c.Tr.Actions.Undo,
-				Prompt: fmt.Sprintf(self.c.Tr.SoftResetPrompt, action.from),
+				Prompt: fmt.Sprintf(self.c.Tr.SoftResetPrompt, utils.ShortHash(action.from)),
 				HandleConfirm: func() error {
 					self.c.LogAction(self.c.Tr.Actions.Undo)
 					return self.c.WithWaitingStatus(undoingStatus, func(gocui.Task) error {
@@ -103,7 +103,7 @@ func (self *UndoController) reflogUndo() error {
 		case REBASE:
 			self.c.Confirm(types.ConfirmOpts{
 				Title:  self.c.Tr.Actions.Undo,
-				Prompt: fmt.Sprintf(self.c.Tr.HardResetAutostashPrompt, action.from),
+				Prompt: fmt.Sprintf(self.c.Tr.HardResetAutostashPrompt, utils.ShortHash(action.from)),
 				HandleConfirm: func() error {
 					self.c.LogAction(self.c.Tr.Actions.Undo)
 					return self.hardResetWithAutoStash(action.from, hardResetOptions{
@@ -157,7 +157,7 @@ func (self *UndoController) reflogRedo() error {
 		case COMMIT, REBASE:
 			self.c.Confirm(types.ConfirmOpts{
 				Title:  self.c.Tr.Actions.Redo,
-				Prompt: fmt.Sprintf(self.c.Tr.HardResetAutostashPrompt, action.to),
+				Prompt: fmt.Sprintf(self.c.Tr.HardResetAutostashPrompt, utils.ShortHash(action.to)),
 				HandleConfirm: func() error {
 					self.c.LogAction(self.c.Tr.Actions.Redo)
 					return self.hardResetWithAutoStash(action.to, hardResetOptions{
@@ -260,7 +260,7 @@ func (self *UndoController) hardResetWithAutoStash(commitHash string, options ha
 	dirtyWorkingTree := self.c.Helpers().WorkingTree.IsWorkingTreeDirty()
 	if dirtyWorkingTree {
 		return self.c.WithWaitingStatus(options.WaitingStatus, func(gocui.Task) error {
-			if err := self.c.Git().Stash.Push(self.c.Tr.StashPrefix + commitHash); err != nil {
+			if err := self.c.Git().Stash.Push(fmt.Sprintf(self.c.Tr.AutoStashForUndo, utils.ShortHash(commitHash))); err != nil {
 				return err
 			}
 			if err := reset(); err != nil {

pkg/i18n/english.go

@@ -447,7 +447,12 @@ type TranslationSet struct {
 	IncorrectNotARepository               string
 	AutoStashTitle                        string
 	AutoStashPrompt                       string
-	StashPrefix                           string
+	AutoStashForUndo                      string
+	AutoStashForCheckout                  string
+	AutoStashForNewBranch                 string
+	AutoStashForMovingPatchToIndex        string
+	AutoStashForCherryPicking             string
+	AutoStashForReverting                 string
 	Discard                               string
 	DiscardChangesTitle                   string
 	DiscardFileChangesTooltip             string
@@ -1540,7 +1545,12 @@ func EnglishTranslationSet() *TranslationSet {
 		IncorrectNotARepository:              "The value of 'notARepository' is incorrect. It should be one of 'prompt', 'create', 'skip', or 'quit'.",
 		AutoStashTitle:                       "Autostash?",
 		AutoStashPrompt:                      "You must stash and pop your changes to bring them across. Do this automatically? (enter/esc)",
-		StashPrefix:                          "Auto-stashing changes for ",
+		AutoStashForUndo:                     "Auto-stashing changes for undoing to %s",
+		AutoStashForCheckout:                 "Auto-stashing changes for checking out %s",
+		AutoStashForNewBranch:                "Auto-stashing changes for creating new branch %s",
+		AutoStashForMovingPatchToIndex:       "Auto-stashing changes for moving custom patch to index from %s",
+		AutoStashForCherryPicking:            "Auto-stashing changes for cherry-picking commits",
+		AutoStashForReverting:                "Auto-stashing changes for reverting commits",
 		Discard:                              "Discard",
 		DiscardFileChangesTooltip:            "View options for discarding changes to the selected file.",
 		DiscardChangesTitle:                  "Discard changes",

pkg/integration/tests/cherry_pick/cherry_pick.go

@@ -75,7 +75,7 @@ var CherryPick = NewIntegrationTest(NewIntegrationTestArgs{
 			Lines(
 				Contains("four"),
 				Contains("three"),
-				Contains("two"),
+				Contains("two").IsSelected(),
 				Contains("one"),
 				Contains("base"),
 			)
@@ -104,7 +104,7 @@ var CherryPick = NewIntegrationTest(NewIntegrationTestArgs{
 			Lines(
 				Contains("four"),
 				Contains("three"),
-				Contains("base"),
+				Contains("base").IsSelected(),
 			)
 	},
 })

pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go

@@ -43,7 +43,7 @@ var CherryPickConflicts = NewIntegrationTest(NewIntegrationTestArgs{
 		t.Views().Commits().
 			Focus().
 			TopLines(
-				Contains("first change"),
+				Contains("first change").IsSelected(),
 			).
 			Press(keys.Commits.PasteCommits)
 
@@ -76,7 +76,7 @@ var CherryPickConflicts = NewIntegrationTest(NewIntegrationTestArgs{
 		t.Views().Commits().
 			Focus().
 			TopLines(
-				Contains("second-change-branch unrelated change"),
+				Contains("second-change-branch unrelated change").IsSelected(),
 				Contains("second change"),
 				Contains("first change"),
 			).

pkg/integration/tests/cherry_pick/cherry_pick_during_rebase.go

@@ -80,7 +80,7 @@ var CherryPickDuringRebase = NewIntegrationTest(NewIntegrationTestArgs{
 				Contains("pick CI two"),
 				Contains("--- Commits ---"),
 				Contains("     CI three"),
-				Contains("     CI one"),
+				Contains("     CI one").IsSelected(),
 				Contains("     CI base"),
 			).
 			Tap(func() {
@@ -89,7 +89,7 @@ var CherryPickDuringRebase = NewIntegrationTest(NewIntegrationTestArgs{
 			Lines(
 				Contains("CI two"),
 				Contains("CI three"),
-				Contains("CI one"),
+				Contains("CI one").IsSelected(),
 				Contains("CI base"),
 			)
 	},

pkg/integration/tests/cherry_pick/cherry_pick_merge.go

@@ -63,9 +63,10 @@ var CherryPickMerge = NewIntegrationTest(NewIntegrationTestArgs{
 				t.Views().Information().Content(DoesNotContain("commit copied"))
 			}).
 			Lines(
-				Contains("Merge branch 'second-branch'").IsSelected(),
+				Contains("Merge branch 'second-branch'"),
-				Contains("base"),
+				Contains("base").IsSelected(),
-			)
+			).
+			SelectPreviousItem()
 
 		t.Views().Main().ContainsLines(
 			Contains("Merge branch 'second-branch'"),

pkg/integration/tests/cherry_pick/cherry_pick_range.go

@@ -72,7 +72,7 @@ var CherryPickRange = NewIntegrationTest(NewIntegrationTestArgs{
 			Lines(
 				Contains("four"),
 				Contains("three"),
-				Contains("two"),
+				Contains("two").IsSelected(),
 				Contains("one"),
 				Contains("base"),
 			)

pkg/integration/tests/commit/revert.go

@@ -29,9 +29,10 @@ var Revert = NewIntegrationTest(NewIntegrationTestArgs{
 					Confirm()
 			}).
 			Lines(
-				Contains("Revert \"first commit\"").IsSelected(),
+				Contains("Revert \"first commit\""),
-				Contains("first commit"),
+				Contains("first commit").IsSelected(),
-			)
+			).
+			SelectPreviousItem()
 
 		t.Views().Main().Content(Contains("-myfile content"))
 		t.FileSystem().PathNotPresent("myfile")

pkg/integration/tests/reflog/cherry_pick.go

@@ -43,8 +43,8 @@ var CherryPick = NewIntegrationTest(NewIntegrationTestArgs{
 					Confirm()
 			}).
 			Lines(
-				Contains("three").IsSelected(),
+				Contains("three"),
-				Contains("one"),
+				Contains("one").IsSelected(),
 			)
 	},
 })