Show an error when checking out a file would overwrite local modifications (#5154)

Checking out a file discards all local modifications to that file;
checking out a directory discards modifications to all files in that
directory. This can result in lost work if the user isn't aware of this.

Instead of showing a confirmation (to which the user might press enter
too hastily, still losing their work), or showing a menu with
auto-stash/discard options, we simply show an error. The assumption is
that this is an unusual situation that users won't run into often, and
if they do, it's easy enough to manually address it by stashing or
discarding the changes as appropriate.

Fixes #5142.

Author: stefanhaller

pkg/gui/controllers/commits_files_controller.go

@@ -12,6 +12,7 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/commands/patch"
 	"github.com/jesseduffield/lazygit/pkg/constants"
 	"github.com/jesseduffield/lazygit/pkg/gui/context"
+	"github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers"
 	"github.com/jesseduffield/lazygit/pkg/gui/filetree"
 	"github.com/jesseduffield/lazygit/pkg/gui/keybindings"
 	"github.com/jesseduffield/lazygit/pkg/gui/types"
@@ -283,6 +284,12 @@ func (self *CommitFilesController) openCopyMenu() error {
 }
 
 func (self *CommitFilesController) checkout(node *filetree.CommitFileNode) error {
+	hasModifiedFiles := helpers.AnyTrackedFilesInPathExceptSubmodules(node.GetPath(),
+		self.c.Model().Files, self.c.Model().Submodules)
+	if hasModifiedFiles {
+		return errors.New(self.c.Tr.CannotCheckoutWithModifiedFilesErr)
+	}
+
 	self.c.LogAction(self.c.Tr.Actions.CheckoutFile)
 	_, to := self.context().GetFromAndToForDiff()
 	if err := self.c.Git().WorkingTree.CheckoutFile(to, node.GetPath()); err != nil {

pkg/gui/controllers/helpers/working_tree_helper.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"regexp"
+	"strings"
 
 	"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
@@ -71,6 +72,23 @@ func AnyTrackedFilesExceptSubmodules(files []*models.File, submoduleConfigs []*m
 	return lo.SomeBy(files, func(f *models.File) bool { return f.Tracked && !f.IsSubmodule(submoduleConfigs) })
 }
 
+func isContainedInPath(candidate string, path string) bool {
+	return (
+	// If the path is the repo root (appears as "/" in the UI), then all candidates are contained in it
+	path == "." ||
+		// Exact match; will only be true for files
+		candidate == path ||
+		// Match for files within a directory. We need to match the trailing slash to avoid
+		// matching files with longer names.
+		strings.HasPrefix(candidate, path+"/"))
+}
+
+func AnyTrackedFilesInPathExceptSubmodules(path string, files []*models.File, submoduleConfigs []*models.SubmoduleConfig) bool {
+	return lo.SomeBy(files, func(f *models.File) bool {
+		return f.Tracked && isContainedInPath(f.GetPath(), path) && !f.IsSubmodule(submoduleConfigs)
+	})
+}
+
 func (self *WorkingTreeHelper) IsWorkingTreeDirtyExceptSubmodules() bool {
 	return IsWorkingTreeDirtyExceptSubmodules(self.c.Model().Files, self.c.Model().Submodules)
 }

pkg/i18n/english.go

@@ -425,6 +425,7 @@ type TranslationSet struct {
 	ViewItemFiles                         string
 	CommitFilesTitle                      string
 	CheckoutCommitFileTooltip             string
+	CannotCheckoutWithModifiedFilesErr    string
 	CanOnlyDiscardFromLocalCommits        string
 	Remove                                string
 	DiscardOldFileChangeTooltip           string
@@ -1525,6 +1526,7 @@ func EnglishTranslationSet() *TranslationSet {
 		ViewItemFiles:                        "View files",
 		CommitFilesTitle:                     "Commit files",
 		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",
 		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.",

pkg/integration/tests/commit/checkout_file_with_local_modifications.go

@@ -0,0 +1,40 @@
+package commit
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var CheckoutFileWithLocalModifications = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Checkout a file from a commit that has local modifications",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.CreateFileAndAdd("dir/file1.txt", "file1\n")
+		shell.CreateFileAndAdd("dir/file2.txt", "file2\n")
+		shell.Commit("one")
+		shell.UpdateFile("dir/file1.txt", "file1\nfile1 change\n")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("one").IsSelected(),
+			).
+			PressEnter()
+
+		t.Views().CommitFiles().
+			IsFocused().
+			Lines(
+				Equals("▼ dir").IsSelected(),
+				Equals("  A file1.txt"),
+				Equals("  A file2.txt"),
+			).
+			Press(keys.CommitFiles.CheckoutCommitFile)
+
+		t.ExpectPopup().Alert().Title(Equals("Error")).
+			Content(Contains("local modifications")).
+			Confirm()
+	},
+})

pkg/integration/tests/test_list.go

@@ -107,6 +107,7 @@ var tests = []*components.IntegrationTest{
 	commit.Checkout,
 	commit.CheckoutFileFromCommit,
 	commit.CheckoutFileFromRangeSelectionOfCommits,
+	commit.CheckoutFileWithLocalModifications,
 	commit.Commit,
 	commit.CommitMultiline,
 	commit.CommitSkipHooks,