Support range select removing files from a commit

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)
+	for _, filePath := range filePaths {
-	cmdArgs := NewGitCmd("cat-file").Arg("-e", "HEAD^:"+fileName).ToArgv()
+		// 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.cmd.New(cmdArgs).Run(); err != nil {
-		if err := self.os.Remove(fileName); 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

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).

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),
+			Handler:           self.withItems(self.discard),
-			GetDisabledReason: self.require(self.singleItemSelected()),
+			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
+	removeFileRange := func() error {
-	if node.File.Added() {
+		return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error {
-		prompt = self.c.Tr.DiscardAddedFileChangesPrompt
+			selectedNodes = normalisedSelectedCommitFileNodes(selectedNodes)
-	} else if node.File.Deleted() {
-		prompt = self.c.Tr.DiscardDeletedFileChangesPrompt
-	}
 
-	return self.c.Confirm(types.ConfirmOpts{
+			return self.c.Confirm(types.ConfirmOpts{
-		Title:  self.c.Tr.DiscardFileChangesTitle,
+				Title:  self.c.Tr.DiscardFileChangesTitle,
-		Prompt: prompt,
+				Prompt: self.c.Tr.DiscardFileChangesPrompt,
-		HandleConfirm: func() error {
+				HandleConfirm: func() error {
-			return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error {
+					var filePaths []string
-				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 {
+					// 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 {

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?",
+		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.",
-		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.",
 		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}}.",

pkg/i18n/russian.go

@@ -289,10 +289,7 @@ func RussianTranslationSet() TranslationSet {
 		CanOnlyDiscardFromLocalCommits:      "Изменения можно отменить только из локальных коммитов.",
 		DiscardOldFileChangeTooltip:         "Отменить изменения коммита в этом файле",
 		DiscardFileChangesTitle:             "Отменить изменения файла",
-		DiscardFileChangesPrompt:            "Вы уверены, что хотите отменить изменения коммита в этом файле? Если файл был создан в этом коммите, он будет удалён",
+		DiscardFileChangesPrompt:            "Вы уверены, что хотите удалить изменения в выбранных файлах из этого коммита?\n\nЭто действие запустит перебазирование и отменит изменения в этих файлах. Обратите внимание, что если последующие коммиты зависят от этих изменений, вам, возможно, придется разрешить конфликты.\nПримечание: это также сбросит все активные пользовательские патчи.",
-		DiscardAddedFileChangesPrompt:       "Вы уверены, что хотите отменить изменения, внесённые в этот файл коммитом? Файл был добавлен в этот коммит, поэтому он снова будет удален.",
-		DiscardDeletedFileChangesPrompt:     "Вы уверены, что хотите отменить изменения, внесённые в этот файл коммитом? Файл был удалён в этом коммите, поэтому он снова появится.",
-		DiscardNotSupportedForDirectory:     "Отмена изменений не поддерживается для всех каталогов. Используйте для этого специальный патч.",
 		DisabledForGPG:                      "Функция недоступна для пользователей, использующих GPG",
 		CreateRepo:                          "Не в git репозитории. Создать новый git репозиторий? (y/n):",
 		BareRepo:                            "Вы пытались открыть Lazygit в пустом репозитории, но Lazygit ещё не поддерживает пустые репозитории. Открыть последний репозиторий? (y/n)",

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().

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)"),
+			)
+	},
+})

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,