From 15d5261933912f0665ca1920f5131dce9cc2dbb3 Mon Sep 17 00:00:00 2001
From: Aaron Hoffman <kd0sgh@gmail.com>
Date: Sun, 28 Jan 2024 09:15:29 -0600
Subject: [PATCH 1/4] Support range select removing files from a commit

---
 pkg/commands/git_commands/rebase.go           |  22 +--
 pkg/commands/git_commands/rebase_test.go      |   8 +-
 .../controllers/commits_files_controller.go   |  61 ++++----
 pkg/i18n/english.go                           |  10 +-
 pkg/i18n/russian.go                           |   5 +-
 .../tests/commit/discard_old_file_change.go   |   2 +-
 .../tests/commit/discard_range_select.go      | 131 ++++++++++++++++++
 pkg/integration/tests/test_list.go            |   1 +
 8 files changed, 190 insertions(+), 50 deletions(-)
 create mode 100644 pkg/integration/tests/commit/discard_range_select.go

diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go
index 03d4030ac..0bcfa5f67 100644
--- a/pkg/commands/git_commands/rebase.go
+++ b/pkg/commands/git_commands/rebase.go
@@ -423,23 +423,25 @@ func (self *RebaseCommands) runSkipEditorCommand(cmdObj oscommands.ICmdObj) erro
 }
 
 // DiscardOldFileChanges discards changes to a file from an old commit
-func (self *RebaseCommands) DiscardOldFileChanges(commits []*models.Commit, commitIndex int, fileName string) error {
+func (self *RebaseCommands) DiscardOldFileChanges(commits []*models.Commit, commitIndex int, filePaths []string) error {
 	if err := self.BeginInteractiveRebaseForCommit(commits, commitIndex, false); err != nil {
 		return err
 	}
 
-	// check if file exists in previous commit (this command returns an error if the file doesn't exist)
-	cmdArgs := NewGitCmd("cat-file").Arg("-e", "HEAD^:"+fileName).ToArgv()
+	for _, filePath := range filePaths {
+		// check if file exists in previous commit (this command returns an error if the file doesn't exist)
+		cmdArgs := NewGitCmd("cat-file").Arg("-e", "HEAD^:"+filePath).ToArgv()
 
-	if err := self.cmd.New(cmdArgs).Run(); err != nil {
-		if err := self.os.Remove(fileName); err != nil {
+		if err := self.cmd.New(cmdArgs).Run(); err != nil {
+			if err := self.os.Remove(filePath); err != nil {
+				return err
+			}
+			if err := self.workingTree.StageFile(filePath); err != nil {
+				return err
+			}
+		} else if err := self.workingTree.CheckoutFile("HEAD^", filePath); err != nil {
 			return err
 		}
-		if err := self.workingTree.StageFile(fileName); err != nil {
-			return err
-		}
-	} else if err := self.workingTree.CheckoutFile("HEAD^", fileName); err != nil {
-		return err
 	}
 
 	// amend the commit
diff --git a/pkg/commands/git_commands/rebase_test.go b/pkg/commands/git_commands/rebase_test.go
index d10746220..b84621497 100644
--- a/pkg/commands/git_commands/rebase_test.go
+++ b/pkg/commands/git_commands/rebase_test.go
@@ -111,7 +111,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
 		gitConfigMockResponses map[string]string
 		commits                []*models.Commit
 		commitIndex            int
-		fileName               string
+		fileName               []string
 		runner                 *oscommands.FakeCmdObjRunner
 		test                   func(error)
 	}
@@ -122,7 +122,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
 			gitConfigMockResponses: nil,
 			commits:                []*models.Commit{},
 			commitIndex:            0,
-			fileName:               "test999.txt",
+			fileName:               []string{"test999.txt"},
 			runner:                 oscommands.NewFakeRunner(t),
 			test: func(err error) {
 				assert.Error(t, err)
@@ -133,7 +133,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
 			gitConfigMockResponses: map[string]string{"commit.gpgsign": "true"},
 			commits:                []*models.Commit{{Name: "commit", Sha: "123456"}},
 			commitIndex:            0,
-			fileName:               "test999.txt",
+			fileName:               []string{"test999.txt"},
 			runner:                 oscommands.NewFakeRunner(t),
 			test: func(err error) {
 				assert.Error(t, err)
@@ -147,7 +147,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
 				{Name: "commit2", Sha: "abcdef"},
 			},
 			commitIndex: 0,
-			fileName:    "test999.txt",
+			fileName:    []string{"test999.txt"},
 			runner: oscommands.NewFakeRunner(t).
 				ExpectGitArgs([]string{"rebase", "--interactive", "--autostash", "--keep-empty", "--no-autosquash", "--rebase-merges", "abcdef"}, "", nil).
 				ExpectGitArgs([]string{"cat-file", "-e", "HEAD^:test999.txt"}, "", nil).
diff --git a/pkg/gui/controllers/commits_files_controller.go b/pkg/gui/controllers/commits_files_controller.go
index 647d6594b..326a8a6d3 100644
--- a/pkg/gui/controllers/commits_files_controller.go
+++ b/pkg/gui/controllers/commits_files_controller.go
@@ -50,8 +50,8 @@ func (self *CommitFilesController) GetKeybindings(opts types.KeybindingsOpts) []
 		},
 		{
 			Key:               opts.GetKey(opts.Config.Universal.Remove),
-			Handler:           self.withItem(self.discard),
-			GetDisabledReason: self.require(self.singleItemSelected()),
+			Handler:           self.withItems(self.discard),
+			GetDisabledReason: self.require(self.itemsSelected()),
 			Description:       self.c.Tr.Remove,
 			Tooltip:           self.c.Tr.DiscardOldFileChangeTooltip,
 			DisplayOnScreen:   true,
@@ -176,43 +176,56 @@ func (self *CommitFilesController) checkout(node *filetree.CommitFileNode) error
 	return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC})
 }
 
-func (self *CommitFilesController) discard(node *filetree.CommitFileNode) error {
+func (self *CommitFilesController) discard(selectedNodes []*filetree.CommitFileNode) error {
 	parentContext, ok := self.c.CurrentContext().GetParentContext()
 	if !ok || parentContext.GetKey() != context.LOCAL_COMMITS_CONTEXT_KEY {
 		return self.c.ErrorMsg(self.c.Tr.CanOnlyDiscardFromLocalCommits)
 	}
 
-	if node.File == nil {
-		return self.c.ErrorMsg(self.c.Tr.DiscardNotSupportedForDirectory)
-	}
-
 	if ok, err := self.c.Helpers().PatchBuilding.ValidateNormalWorkingTreeState(); !ok {
 		return err
 	}
 
-	prompt := self.c.Tr.DiscardFileChangesPrompt
-	if node.File.Added() {
-		prompt = self.c.Tr.DiscardAddedFileChangesPrompt
-	} else if node.File.Deleted() {
-		prompt = self.c.Tr.DiscardDeletedFileChangesPrompt
-	}
+	removeFileRange := func() error {
+		return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error {
+			selectedNodes = normalisedSelectedCommitFileNodes(selectedNodes)
 
-	return self.c.Confirm(types.ConfirmOpts{
-		Title:  self.c.Tr.DiscardFileChangesTitle,
-		Prompt: prompt,
-		HandleConfirm: func() error {
-			return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error {
-				self.c.LogAction(self.c.Tr.Actions.DiscardOldFileChange)
-				if err := self.c.Git().Rebase.DiscardOldFileChanges(self.c.Model().Commits, self.c.Contexts().LocalCommits.GetSelectedLineIdx(), node.GetPath()); err != nil {
+			return self.c.Confirm(types.ConfirmOpts{
+				Title:  self.c.Tr.DiscardFileChangesTitle,
+				Prompt: self.c.Tr.DiscardFileChangesPrompt,
+				HandleConfirm: func() error {
+					var filePaths []string
+
+					// Reset the current patch if there is one.
+					if self.c.Git().Patch.PatchBuilder.Active() {
+						self.c.Git().Patch.PatchBuilder.Reset()
+						if err := self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI}); err != nil {
+							return err
+						}
+					}
+
+					for _, node := range selectedNodes {
+						err := node.ForEachFile(func(file *models.CommitFile) error {
+							filePaths = append(filePaths, file.GetPath())
+							return nil
+						})
+						if err != nil {
+							return self.c.Error(err)
+						}
+					}
+
+					err := self.c.Git().Rebase.DiscardOldFileChanges(self.c.Model().Commits, self.c.Contexts().LocalCommits.GetSelectedLineIdx(), filePaths)
 					if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil {
 						return err
 					}
-				}
 
-				return self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI})
+					return self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI})
+				},
 			})
-		},
-	})
+		})
+	}
+
+	return removeFileRange()
 }
 
 func (self *CommitFilesController) open(node *filetree.CommitFileNode) error {
diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go
index 048f0e6c4..da8dbc809 100644
--- a/pkg/i18n/english.go
+++ b/pkg/i18n/english.go
@@ -354,9 +354,6 @@ type TranslationSet struct {
 	DiscardOldFileChangeTooltip           string
 	DiscardFileChangesTitle               string
 	DiscardFileChangesPrompt              string
-	DiscardAddedFileChangesPrompt         string
-	DiscardDeletedFileChangesPrompt       string
-	DiscardNotSupportedForDirectory       string
 	DisabledForGPG                        string
 	CreateRepo                            string
 	BareRepo                              string
@@ -420,6 +417,7 @@ type TranslationSet struct {
 	ScrollRight                           string
 	DiscardPatch                          string
 	DiscardPatchConfirm                   string
+	DiscardPatchSameCommitConfirm         string
 	CantPatchWhileRebasingError           string
 	ToggleAddToPatch                      string
 	ToggleAddToPatchTooltip               string
@@ -1295,10 +1293,7 @@ func EnglishTranslationSet() TranslationSet {
 		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.",
 		DiscardFileChangesTitle:             "Discard file changes",
-		DiscardFileChangesPrompt:            "Are you sure you want to discard this commit's changes to this file?",
-		DiscardAddedFileChangesPrompt:       "Are you sure you want to discard this commit's changes to this file? The file was added in this commit, so it will be deleted again.",
-		DiscardDeletedFileChangesPrompt:     "Are you sure you want to discard this commit's changes to this file? The file was deleted in this commit, so it will reappear.",
-		DiscardNotSupportedForDirectory:     "Discarding changes is not supported for entire directories. Please use a custom patch for this.",
+		DiscardFileChangesPrompt:            "Are you sure you want to remove changes to the selected file(s) from this commit?\n\nThis action will start a rebase, reverting these file changes. Be aware that if subsequent commits depend on these changes, you may need to resolve conflicts.\nNote: This will also reset any active custom patches.",
 		DisabledForGPG:                      "Feature not available for users using GPG",
 		CreateRepo:                          "Not in a git repository. Create a new git repository? (y/n): ",
 		BareRepo:                            "You've attempted to open Lazygit in a bare repo but Lazygit does not yet support bare repos. Open most recent repo? (y/n) ",
@@ -1363,6 +1358,7 @@ func EnglishTranslationSet() TranslationSet {
 		ScrollRight:                         "Scroll right",
 		DiscardPatch:                        "Discard patch",
 		DiscardPatchConfirm:                 "You can only build a patch from one commit/stash-entry at a time. Discard current patch?",
+		DiscardPatchSameCommitConfirm:       "You currently have changes added to a patch for this commit. Discard current patch?",
 		CantPatchWhileRebasingError:         "You cannot build a patch or run patch commands while in a merging or rebasing state",
 		ToggleAddToPatch:                    "Toggle file included in patch",
 		ToggleAddToPatchTooltip:             "Toggle whether the file is included in the custom patch. See {{.doc}}.",
diff --git a/pkg/i18n/russian.go b/pkg/i18n/russian.go
index af3fdd6bd..e27ebabff 100644
--- a/pkg/i18n/russian.go
+++ b/pkg/i18n/russian.go
@@ -289,10 +289,7 @@ func RussianTranslationSet() TranslationSet {
 		CanOnlyDiscardFromLocalCommits:      "Изменения можно отменить только из локальных коммитов.",
 		DiscardOldFileChangeTooltip:         "Отменить изменения коммита в этом файле",
 		DiscardFileChangesTitle:             "Отменить изменения файла",
-		DiscardFileChangesPrompt:            "Вы уверены, что хотите отменить изменения коммита в этом файле? Если файл был создан в этом коммите, он будет удалён",
-		DiscardAddedFileChangesPrompt:       "Вы уверены, что хотите отменить изменения, внесённые в этот файл коммитом? Файл был добавлен в этот коммит, поэтому он снова будет удален.",
-		DiscardDeletedFileChangesPrompt:     "Вы уверены, что хотите отменить изменения, внесённые в этот файл коммитом? Файл был удалён в этом коммите, поэтому он снова появится.",
-		DiscardNotSupportedForDirectory:     "Отмена изменений не поддерживается для всех каталогов. Используйте для этого специальный патч.",
+		DiscardFileChangesPrompt:            "Вы уверены, что хотите удалить изменения в выбранных файлах из этого коммита?\n\nЭто действие запустит перебазирование и отменит изменения в этих файлах. Обратите внимание, что если последующие коммиты зависят от этих изменений, вам, возможно, придется разрешить конфликты.\nПримечание: это также сбросит все активные пользовательские патчи.",
 		DisabledForGPG:                      "Функция недоступна для пользователей, использующих GPG",
 		CreateRepo:                          "Не в git репозитории. Создать новый git репозиторий? (y/n):",
 		BareRepo:                            "Вы пытались открыть Lazygit в пустом репозитории, но Lazygit ещё не поддерживает пустые репозитории. Открыть последний репозиторий? (y/n)",
diff --git a/pkg/integration/tests/commit/discard_old_file_change.go b/pkg/integration/tests/commit/discard_old_file_change.go
index 5d5fcc0c8..0b215d735 100644
--- a/pkg/integration/tests/commit/discard_old_file_change.go
+++ b/pkg/integration/tests/commit/discard_old_file_change.go
@@ -43,7 +43,7 @@ var DiscardOldFileChange = NewIntegrationTest(NewIntegrationTestArgs{
 
 		t.ExpectPopup().Confirmation().
 			Title(Equals("Discard file changes")).
-			Content(Equals("Are you sure you want to discard this commit's changes to this file? The file was added in this commit, so it will be deleted again.")).
+			Content(Equals("Are you sure you want to remove changes to the selected file(s) from this commit?\n\nThis action will start a rebase, reverting these file changes. Be aware that if subsequent commits depend on these changes, you may need to resolve conflicts.\nNote: This will also reset any active custom patches.")).
 			Confirm()
 
 		t.Views().CommitFiles().
diff --git a/pkg/integration/tests/commit/discard_range_select.go b/pkg/integration/tests/commit/discard_range_select.go
new file mode 100644
index 000000000..65b50a6be
--- /dev/null
+++ b/pkg/integration/tests/commit/discard_range_select.go
@@ -0,0 +1,131 @@
+package commit
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var DiscardRangeSelect = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Discarding a range of files from an old commit.",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.CreateFileAndAdd("dir1/file0", "file0\n")
+		shell.CreateFileAndAdd("dir1/dir2/file1", "file1\n")
+		shell.CreateFileAndAdd("dir3/file1", "d3f1 content\n")
+		shell.CreateFileAndAdd("dir3/file4", "d3f4 content\n")
+		shell.Commit("first commit")
+
+		shell.UpdateFileAndAdd("dir3/file1", "d3f1 content\nsecond line\n")
+		shell.CreateFileAndAdd("dir3/file2", "d3f2 content\n")
+		shell.CreateFileAndAdd("dir3/file3", "d3f3 content\n")
+		shell.DeleteFileAndAdd("dir3/file4")
+		shell.Commit("first commit to change")
+
+		shell.CreateFileAndAdd("dir1/fileToRemove", "file to remove content\n")
+		shell.CreateFileAndAdd("dir1/multiLineFile", "this file has\ncontent on\nthree lines\n")
+		shell.CreateFileAndAdd("dir1/dir2/file2ToRemove", "file2 to remove content\n")
+		shell.Commit("second commit to change")
+
+		shell.CreateFileAndAdd("file3", "file3")
+		shell.Commit("third commit")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("third commit").IsSelected(),
+				Contains("second commit to change"),
+				Contains("first commit to change"),
+				Contains("first commit"),
+			).
+			NavigateToLine(Contains("first commit to change")).
+			PressEnter()
+
+		t.Views().CommitFiles().
+			IsFocused().
+			Lines(
+				Contains("dir3").IsSelected(),
+				Contains("file1"),
+				Contains("file2"),
+				Contains("file3"),
+				Contains("file4"),
+			).
+			NavigateToLine(Contains("file1")).
+			Press(keys.Universal.ToggleRangeSelect).
+			NavigateToLine(Contains("file4")).
+			Press(keys.Universal.Remove)
+
+		t.ExpectPopup().Confirmation().
+			Title(Equals("Discard file changes")).
+			Content(Equals("Are you sure you want to remove changes to the selected file(s) from this commit?\n\nThis action will start a rebase, reverting these file changes. Be aware that if subsequent commits depend on these changes, you may need to resolve conflicts.\nNote: This will also reset any active custom patches.")).
+			Confirm()
+
+		t.Views().CommitFiles().
+			IsFocused().
+			Lines(
+				Contains("(none)"),
+			).
+			// for some reason I need to press escape twice. Seems like it happens every time
+			// more than one file is removed from a commit
+			PressEscape().
+			PressEscape()
+
+		t.Views().Commits().
+			IsFocused().
+			Lines(
+				Contains("third commit"),
+				Contains("second commit to change"),
+				Contains("first commit to change").IsSelected(),
+				Contains("first commit"),
+			).
+			NavigateToLine(Contains("second commit to change")).
+			PressEnter()
+
+		t.Views().CommitFiles().
+			IsFocused().
+			Lines(
+				Contains("dir1").IsSelected(),
+				Contains("dir2"),
+				Contains("file2ToRemove"),
+				Contains("fileToRemove"),
+				Contains("multiLineFile"),
+			).
+			NavigateToLine(Contains("multiLineFile")).
+			PressEnter()
+
+		t.Views().PatchBuilding().
+			IsFocused().
+			SelectedLine(
+				Contains("+this file has"),
+			).
+			PressPrimaryAction().
+			PressEscape()
+
+		t.Views().CommitFiles().
+			IsFocused().
+			Lines(
+				Contains("dir1"),
+				Contains("dir2"),
+				Contains("file2ToRemove"),
+				Contains("fileToRemove"),
+				Contains("multiLineFile").IsSelected(),
+			).
+			NavigateToLine(Contains("dir1")).
+			Press(keys.Universal.ToggleRangeSelect).
+			NavigateToLine(Contains("dir2")).
+			Press(keys.Universal.Remove)
+
+		t.ExpectPopup().Confirmation().
+			Title(Equals("Discard file changes")).
+			Content(Equals("Are you sure you want to remove changes to the selected file(s) from this commit?\n\nThis action will start a rebase, reverting these file changes. Be aware that if subsequent commits depend on these changes, you may need to resolve conflicts.\nNote: This will also reset any active custom patches.")).
+			Confirm()
+
+		t.Views().CommitFiles().
+			IsFocused().
+			Lines(
+				Contains("(none)"),
+			)
+	},
+})
diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go
index cfadde3b1..48a0572b9 100644
--- a/pkg/integration/tests/test_list.go
+++ b/pkg/integration/tests/test_list.go
@@ -71,6 +71,7 @@ var tests = []*components.IntegrationTest{
 	commit.CommitWithPrefix,
 	commit.CreateTag,
 	commit.DiscardOldFileChange,
+	commit.DiscardRangeSelect,
 	commit.FindBaseCommitForFixup,
 	commit.FindBaseCommitForFixupWarningForAddedLines,
 	commit.Highlight,

From 62a0c895b47ad7ee0c072a053942af41d214f772 Mon Sep 17 00:00:00 2001
From: Aaron Hoffman <kd0sgh@gmail.com>
Date: Mon, 29 Jan 2024 22:44:21 -0600
Subject: [PATCH 2/4] Cleanup

The waiting status shouldn't happen until after the user has responded
to the popup.

Since we're not giving a standalone prompt about clearing the patch, all
of the business in `discard` doesn't need to be in a function any more
---
 .../controllers/commits_files_controller.go   | 65 +++++++++----------
 1 file changed, 30 insertions(+), 35 deletions(-)

diff --git a/pkg/gui/controllers/commits_files_controller.go b/pkg/gui/controllers/commits_files_controller.go
index 326a8a6d3..2a5b8139a 100644
--- a/pkg/gui/controllers/commits_files_controller.go
+++ b/pkg/gui/controllers/commits_files_controller.go
@@ -186,46 +186,41 @@ func (self *CommitFilesController) discard(selectedNodes []*filetree.CommitFileN
 		return err
 	}
 
-	removeFileRange := func() error {
-		return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error {
-			selectedNodes = normalisedSelectedCommitFileNodes(selectedNodes)
+	return self.c.Confirm(types.ConfirmOpts{
+		Title:  self.c.Tr.DiscardFileChangesTitle,
+		Prompt: self.c.Tr.DiscardFileChangesPrompt,
+		HandleConfirm: func() error {
+			return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error {
+				var filePaths []string
+				selectedNodes = normalisedSelectedCommitFileNodes(selectedNodes)
 
-			return self.c.Confirm(types.ConfirmOpts{
-				Title:  self.c.Tr.DiscardFileChangesTitle,
-				Prompt: self.c.Tr.DiscardFileChangesPrompt,
-				HandleConfirm: func() error {
-					var filePaths []string
-
-					// Reset the current patch if there is one.
-					if self.c.Git().Patch.PatchBuilder.Active() {
-						self.c.Git().Patch.PatchBuilder.Reset()
-						if err := self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI}); err != nil {
-							return err
-						}
-					}
-
-					for _, node := range selectedNodes {
-						err := node.ForEachFile(func(file *models.CommitFile) error {
-							filePaths = append(filePaths, file.GetPath())
-							return nil
-						})
-						if err != nil {
-							return self.c.Error(err)
-						}
-					}
-
-					err := self.c.Git().Rebase.DiscardOldFileChanges(self.c.Model().Commits, self.c.Contexts().LocalCommits.GetSelectedLineIdx(), filePaths)
-					if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil {
+				// Reset the current patch if there is one.
+				if self.c.Git().Patch.PatchBuilder.Active() {
+					self.c.Git().Patch.PatchBuilder.Reset()
+					if err := self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI}); err != nil {
 						return err
 					}
+				}
 
-					return self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI})
-				},
+				for _, node := range selectedNodes {
+					err := node.ForEachFile(func(file *models.CommitFile) error {
+						filePaths = append(filePaths, file.GetPath())
+						return nil
+					})
+					if err != nil {
+						return self.c.Error(err)
+					}
+				}
+
+				err := self.c.Git().Rebase.DiscardOldFileChanges(self.c.Model().Commits, self.c.Contexts().LocalCommits.GetSelectedLineIdx(), filePaths)
+				if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil {
+					return err
+				}
+
+				return self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI})
 			})
-		})
-	}
-
-	return removeFileRange()
+		},
+	})
 }
 
 func (self *CommitFilesController) open(node *filetree.CommitFileNode) error {

From d138f7ce86ec61023c5794f491e362e3fe26e217 Mon Sep 17 00:00:00 2001
From: Aaron Hoffman <kd0sgh@gmail.com>
Date: Tue, 30 Jan 2024 13:52:40 -0600
Subject: [PATCH 3/4] Clean up test case

I'm combining the delete single file case from `discard_old_file_change`
with the content of `discard_range_select` and calling that
`discard_old_file_changes`. Hopefully that cleans things up a little
bit.

This also adds a check that the custom patch is getting reset properly.
---
 .../tests/commit/discard_old_file_change.go   |  57 ------
 .../tests/commit/discard_old_file_changes.go  | 173 ++++++++++++++++++
 .../tests/commit/discard_range_select.go      | 131 -------------
 pkg/integration/tests/test_list.go            |   3 +-
 4 files changed, 174 insertions(+), 190 deletions(-)
 delete mode 100644 pkg/integration/tests/commit/discard_old_file_change.go
 create mode 100644 pkg/integration/tests/commit/discard_old_file_changes.go
 delete mode 100644 pkg/integration/tests/commit/discard_range_select.go

diff --git a/pkg/integration/tests/commit/discard_old_file_change.go b/pkg/integration/tests/commit/discard_old_file_change.go
deleted file mode 100644
index 0b215d735..000000000
--- a/pkg/integration/tests/commit/discard_old_file_change.go
+++ /dev/null
@@ -1,57 +0,0 @@
-package commit
-
-import (
-	"github.com/jesseduffield/lazygit/pkg/config"
-	. "github.com/jesseduffield/lazygit/pkg/integration/components"
-)
-
-var DiscardOldFileChange = NewIntegrationTest(NewIntegrationTestArgs{
-	Description:  "Discarding a single file from an old commit (does rebase in background to remove the file but retain the other one)",
-	ExtraCmdArgs: []string{},
-	Skip:         false,
-	SetupConfig:  func(config *config.AppConfig) {},
-	SetupRepo: func(shell *Shell) {
-		shell.CreateFileAndAdd("file0", "file0")
-		shell.Commit("first commit")
-
-		shell.CreateFileAndAdd("file1", "file2")
-		shell.CreateFileAndAdd("fileToRemove", "fileToRemove")
-		shell.Commit("commit to change")
-
-		shell.CreateFileAndAdd("file3", "file3")
-		shell.Commit("third commit")
-	},
-	Run: func(t *TestDriver, keys config.KeybindingConfig) {
-		t.Views().Commits().
-			Focus().
-			Lines(
-				Contains("third commit").IsSelected(),
-				Contains("commit to change"),
-				Contains("first commit"),
-			).
-			SelectNextItem().
-			PressEnter()
-
-		t.Views().CommitFiles().
-			IsFocused().
-			Lines(
-				Contains("file1").IsSelected(),
-				Contains("fileToRemove"),
-			).
-			SelectNextItem().
-			Press(keys.Universal.Remove)
-
-		t.ExpectPopup().Confirmation().
-			Title(Equals("Discard file changes")).
-			Content(Equals("Are you sure you want to remove changes to the selected file(s) from this commit?\n\nThis action will start a rebase, reverting these file changes. Be aware that if subsequent commits depend on these changes, you may need to resolve conflicts.\nNote: This will also reset any active custom patches.")).
-			Confirm()
-
-		t.Views().CommitFiles().
-			IsFocused().
-			Lines(
-				Contains("file1").IsSelected(),
-			)
-
-		t.FileSystem().PathNotPresent("fileToRemove")
-	},
-})
diff --git a/pkg/integration/tests/commit/discard_old_file_changes.go b/pkg/integration/tests/commit/discard_old_file_changes.go
new file mode 100644
index 000000000..4b97a30b7
--- /dev/null
+++ b/pkg/integration/tests/commit/discard_old_file_changes.go
@@ -0,0 +1,173 @@
+package commit
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var DiscardOldFileChanges = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Discarding a range of files from an old commit.",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.CreateFileAndAdd("dir1/d1_file0", "file0\n")
+		shell.CreateFileAndAdd("dir1/subd1/subfile0", "file1\n")
+		shell.CreateFileAndAdd("dir2/d2_file1", "d2f1 content\n")
+		shell.CreateFileAndAdd("dir2/d2_file2", "d2f4 content\n")
+		shell.Commit("remove one file from this commit")
+
+		shell.UpdateFileAndAdd("dir2/d2_file1", "d2f1 content\nsecond line\n")
+		shell.DeleteFileAndAdd("dir2/d2_file2")
+		shell.CreateFileAndAdd("dir2/d2_file3", "d2f3 content\n")
+		shell.CreateFileAndAdd("dir2/d2_file4", "d2f2 content\n")
+		shell.Commit("remove four files from this commit")
+
+		shell.CreateFileAndAdd("dir1/fileToRemove", "file to remove content\n")
+		shell.CreateFileAndAdd("dir1/multiLineFile", "this file has\ncontent on\nthree lines\n")
+		shell.CreateFileAndAdd("dir1/subd1/file2ToRemove", "file2 to remove content\n")
+		shell.Commit("remove changes in multiple dirs from this commit")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("remove changes in multiple dirs from this commit").IsSelected(),
+				Contains("remove four files from this commit"),
+				Contains("remove one file from this commit"),
+			).
+			NavigateToLine(Contains("remove one file from this commit")).
+			PressEnter()
+
+		// Check removing a single file from an old commit
+		t.Views().CommitFiles().
+			IsFocused().
+			Lines(
+				Contains("dir1").IsSelected(),
+				Contains("subd1"),
+				Contains("subfile0"),
+				Contains("d1_file0"),
+				Contains("dir2"),
+				Contains("d2_file1"),
+				Contains("d2_file2"),
+			).
+			NavigateToLine(Contains("d1_file0")).
+			Press(keys.Universal.Remove)
+
+		t.ExpectPopup().Confirmation().
+			Title(Equals("Discard file changes")).
+			Content(Equals("Are you sure you want to remove changes to the selected file(s) from this commit?\n\nThis action will start a rebase, reverting these file changes. Be aware that if subsequent commits depend on these changes, you may need to resolve conflicts.\nNote: This will also reset any active custom patches.")).
+			Confirm()
+
+		t.Views().CommitFiles().
+			IsFocused().
+			Lines(
+				Contains("dir1/subd1"),
+				Contains("subfile0"),
+				Contains("dir2"),
+				Contains("d2_file1").IsSelected(),
+				Contains("d2_file2"),
+			).
+			PressEscape()
+
+		// Check removing 4 files in the same directory
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("remove changes in multiple dirs from this commit"),
+				Contains("remove four files from this commit"),
+				Contains("remove one file from this commit").IsSelected(),
+			).
+			NavigateToLine(Contains("remove four files from this commit")).
+			PressEnter()
+
+		t.Views().CommitFiles().
+			IsFocused().
+			Lines(
+				Contains("dir2").IsSelected(),
+				Contains("d2_file1"),
+				Contains("d2_file2"),
+				Contains("d2_file3"),
+				Contains("d2_file4"),
+			).
+			NavigateToLine(Contains("d2_file1")).
+			Press(keys.Universal.ToggleRangeSelect).
+			NavigateToLine(Contains("d2_file4")).
+			Press(keys.Universal.Remove)
+
+		t.ExpectPopup().Confirmation().
+			Title(Equals("Discard file changes")).
+			Content(Equals("Are you sure you want to remove changes to the selected file(s) from this commit?\n\nThis action will start a rebase, reverting these file changes. Be aware that if subsequent commits depend on these changes, you may need to resolve conflicts.\nNote: This will also reset any active custom patches.")).
+			Confirm()
+
+		t.Views().CommitFiles().
+			IsFocused().
+			Lines(
+				Contains("(none)"),
+			).
+			// for some reason I need to press escape twice. Seems like it happens every time
+			// more than one file is removed from a commit
+			PressEscape().
+			PressEscape()
+
+		// Check removing multiple files from 2 directories w/ a custom patch.
+		// This checks node selection logic & if the custom patch is getting reset.
+		t.Views().Commits().
+			IsFocused().
+			Lines(
+				Contains("remove changes in multiple dirs from this commit"),
+				Contains("remove four files from this commit").IsSelected(),
+				Contains("remove one file from this commit"),
+			).
+			NavigateToLine(Contains("remove changes in multiple dirs from this commit")).
+			PressEnter()
+
+		t.Views().CommitFiles().
+			IsFocused().
+			Lines(
+				Contains("dir1").IsSelected(),
+				Contains("subd1"),
+				Contains("file2ToRemove"),
+				Contains("fileToRemove"),
+				Contains("multiLineFile"),
+			).
+			NavigateToLine(Contains("multiLineFile")).
+			PressEnter()
+
+		t.Views().PatchBuilding().
+			IsFocused().
+			SelectedLine(
+				Contains("+this file has"),
+			).
+			PressPrimaryAction().
+			PressEscape()
+
+		t.Views().CommitFiles().
+			IsFocused().
+			Lines(
+				Contains("dir1"),
+				Contains("subd1"),
+				Contains("file2ToRemove"),
+				Contains("fileToRemove"),
+				Contains("multiLineFile").IsSelected(),
+			).
+			NavigateToLine(Contains("dir1")).
+			Press(keys.Universal.ToggleRangeSelect).
+			NavigateToLine(Contains("subd1")).
+			Press(keys.Universal.Remove)
+
+		t.ExpectPopup().Confirmation().
+			Title(Equals("Discard file changes")).
+			Content(Equals("Are you sure you want to remove changes to the selected file(s) from this commit?\n\nThis action will start a rebase, reverting these file changes. Be aware that if subsequent commits depend on these changes, you may need to resolve conflicts.\nNote: This will also reset any active custom patches.")).
+			Confirm()
+
+		// "Building patch" will still be in this view if the patch isn't reset properly
+		t.Views().Information().Content(DoesNotContain("Building patch"))
+
+		t.Views().CommitFiles().
+			IsFocused().
+			Lines(
+				Contains("(none)"),
+			)
+	},
+})
diff --git a/pkg/integration/tests/commit/discard_range_select.go b/pkg/integration/tests/commit/discard_range_select.go
deleted file mode 100644
index 65b50a6be..000000000
--- a/pkg/integration/tests/commit/discard_range_select.go
+++ /dev/null
@@ -1,131 +0,0 @@
-package commit
-
-import (
-	"github.com/jesseduffield/lazygit/pkg/config"
-	. "github.com/jesseduffield/lazygit/pkg/integration/components"
-)
-
-var DiscardRangeSelect = NewIntegrationTest(NewIntegrationTestArgs{
-	Description:  "Discarding a range of files from an old commit.",
-	ExtraCmdArgs: []string{},
-	Skip:         false,
-	SetupConfig:  func(config *config.AppConfig) {},
-	SetupRepo: func(shell *Shell) {
-		shell.CreateFileAndAdd("dir1/file0", "file0\n")
-		shell.CreateFileAndAdd("dir1/dir2/file1", "file1\n")
-		shell.CreateFileAndAdd("dir3/file1", "d3f1 content\n")
-		shell.CreateFileAndAdd("dir3/file4", "d3f4 content\n")
-		shell.Commit("first commit")
-
-		shell.UpdateFileAndAdd("dir3/file1", "d3f1 content\nsecond line\n")
-		shell.CreateFileAndAdd("dir3/file2", "d3f2 content\n")
-		shell.CreateFileAndAdd("dir3/file3", "d3f3 content\n")
-		shell.DeleteFileAndAdd("dir3/file4")
-		shell.Commit("first commit to change")
-
-		shell.CreateFileAndAdd("dir1/fileToRemove", "file to remove content\n")
-		shell.CreateFileAndAdd("dir1/multiLineFile", "this file has\ncontent on\nthree lines\n")
-		shell.CreateFileAndAdd("dir1/dir2/file2ToRemove", "file2 to remove content\n")
-		shell.Commit("second commit to change")
-
-		shell.CreateFileAndAdd("file3", "file3")
-		shell.Commit("third commit")
-	},
-	Run: func(t *TestDriver, keys config.KeybindingConfig) {
-		t.Views().Commits().
-			Focus().
-			Lines(
-				Contains("third commit").IsSelected(),
-				Contains("second commit to change"),
-				Contains("first commit to change"),
-				Contains("first commit"),
-			).
-			NavigateToLine(Contains("first commit to change")).
-			PressEnter()
-
-		t.Views().CommitFiles().
-			IsFocused().
-			Lines(
-				Contains("dir3").IsSelected(),
-				Contains("file1"),
-				Contains("file2"),
-				Contains("file3"),
-				Contains("file4"),
-			).
-			NavigateToLine(Contains("file1")).
-			Press(keys.Universal.ToggleRangeSelect).
-			NavigateToLine(Contains("file4")).
-			Press(keys.Universal.Remove)
-
-		t.ExpectPopup().Confirmation().
-			Title(Equals("Discard file changes")).
-			Content(Equals("Are you sure you want to remove changes to the selected file(s) from this commit?\n\nThis action will start a rebase, reverting these file changes. Be aware that if subsequent commits depend on these changes, you may need to resolve conflicts.\nNote: This will also reset any active custom patches.")).
-			Confirm()
-
-		t.Views().CommitFiles().
-			IsFocused().
-			Lines(
-				Contains("(none)"),
-			).
-			// for some reason I need to press escape twice. Seems like it happens every time
-			// more than one file is removed from a commit
-			PressEscape().
-			PressEscape()
-
-		t.Views().Commits().
-			IsFocused().
-			Lines(
-				Contains("third commit"),
-				Contains("second commit to change"),
-				Contains("first commit to change").IsSelected(),
-				Contains("first commit"),
-			).
-			NavigateToLine(Contains("second commit to change")).
-			PressEnter()
-
-		t.Views().CommitFiles().
-			IsFocused().
-			Lines(
-				Contains("dir1").IsSelected(),
-				Contains("dir2"),
-				Contains("file2ToRemove"),
-				Contains("fileToRemove"),
-				Contains("multiLineFile"),
-			).
-			NavigateToLine(Contains("multiLineFile")).
-			PressEnter()
-
-		t.Views().PatchBuilding().
-			IsFocused().
-			SelectedLine(
-				Contains("+this file has"),
-			).
-			PressPrimaryAction().
-			PressEscape()
-
-		t.Views().CommitFiles().
-			IsFocused().
-			Lines(
-				Contains("dir1"),
-				Contains("dir2"),
-				Contains("file2ToRemove"),
-				Contains("fileToRemove"),
-				Contains("multiLineFile").IsSelected(),
-			).
-			NavigateToLine(Contains("dir1")).
-			Press(keys.Universal.ToggleRangeSelect).
-			NavigateToLine(Contains("dir2")).
-			Press(keys.Universal.Remove)
-
-		t.ExpectPopup().Confirmation().
-			Title(Equals("Discard file changes")).
-			Content(Equals("Are you sure you want to remove changes to the selected file(s) from this commit?\n\nThis action will start a rebase, reverting these file changes. Be aware that if subsequent commits depend on these changes, you may need to resolve conflicts.\nNote: This will also reset any active custom patches.")).
-			Confirm()
-
-		t.Views().CommitFiles().
-			IsFocused().
-			Lines(
-				Contains("(none)"),
-			)
-	},
-})
diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go
index 48a0572b9..6a448fff4 100644
--- a/pkg/integration/tests/test_list.go
+++ b/pkg/integration/tests/test_list.go
@@ -70,8 +70,7 @@ var tests = []*components.IntegrationTest{
 	commit.CommitWipWithPrefix,
 	commit.CommitWithPrefix,
 	commit.CreateTag,
-	commit.DiscardOldFileChange,
-	commit.DiscardRangeSelect,
+	commit.DiscardOldFileChanges,
 	commit.FindBaseCommitForFixup,
 	commit.FindBaseCommitForFixupWarningForAddedLines,
 	commit.Highlight,

From c431698dba30ca7d3df09ec560d452cc77891ded Mon Sep 17 00:00:00 2001
From: Aaron Hoffman <kd0sgh@gmail.com>
Date: Tue, 30 Jan 2024 21:56:12 -0600
Subject: [PATCH 4/4] Fix range select bug

After discarding file changes from the commit, the was still referencing
these indexes as being part of the range select. The consequence was
needing to hit escape twice to exit commit files in some situations.
Canceling the range select after discarding changes fixes that.
---
 pkg/gui/controllers/commits_files_controller.go          | 5 ++++-
 pkg/integration/tests/commit/discard_old_file_changes.go | 3 ---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/pkg/gui/controllers/commits_files_controller.go b/pkg/gui/controllers/commits_files_controller.go
index 2a5b8139a..59e0a7f4e 100644
--- a/pkg/gui/controllers/commits_files_controller.go
+++ b/pkg/gui/controllers/commits_files_controller.go
@@ -217,7 +217,10 @@ func (self *CommitFilesController) discard(selectedNodes []*filetree.CommitFileN
 					return err
 				}
 
-				return self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI})
+				if self.context().RangeSelectEnabled() {
+					self.context().GetList().CancelRangeSelect()
+				}
+				return self.c.Refresh(types.RefreshOptions{Mode: types.SYNC})
 			})
 		},
 	})
diff --git a/pkg/integration/tests/commit/discard_old_file_changes.go b/pkg/integration/tests/commit/discard_old_file_changes.go
index 4b97a30b7..1ab189264 100644
--- a/pkg/integration/tests/commit/discard_old_file_changes.go
+++ b/pkg/integration/tests/commit/discard_old_file_changes.go
@@ -105,9 +105,6 @@ var DiscardOldFileChanges = NewIntegrationTest(NewIntegrationTestArgs{
 			Lines(
 				Contains("(none)"),
 			).
-			// for some reason I need to press escape twice. Seems like it happens every time
-			// more than one file is removed from a commit
-			PressEscape().
 			PressEscape()
 
 		// Check removing multiple files from 2 directories w/ a custom patch.