Fix discarding submodule changes in nested folders (#4317)

- **PR Description**

This PR addresses https://github.com/jesseduffield/lazygit/issues/3951.

The current rules for discarding submodule changes is that no other
changed item must be also selected. There are some bugs with the current
implementation when submodules are in folders. The filed issue goes into
more detail.

As part of this PR, I also tentatively changed the disabled message
("Range select not supported for submodules" -> "Multiselection not
supported for submodules"). The former was not quite accurate because
you could select a single line (folder) but the reset action still needs
to be disabled (folder contains submodule change and some other change).
Not sure if there is some better phrasing.

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/gookit/color v1.4.2
 	github.com/imdario/mergo v0.3.11
 	github.com/integrii/flaggy v1.4.0
-	github.com/jesseduffield/generics v0.0.0-20220320043834-727e535cbe68
+	github.com/jesseduffield/generics v0.0.0-20250406224309-4f541cb84918
 	github.com/jesseduffield/go-git/v5 v5.1.2-0.20221018185014-fdd53fef665d
 	github.com/jesseduffield/gocui v0.3.1-0.20250220081214-b376cb0857ac
 	github.com/jesseduffield/kill v0.0.0-20250101124109-e216ddbe133a

go.sum

@@ -182,8 +182,8 @@ github.com/invopop/jsonschema v0.10.0 h1:c1ktzNLBun3LyQQhyty5WE3lulbOdIIyOVlkmDL
 github.com/invopop/jsonschema v0.10.0/go.mod h1:ffZ5Km5SWWRAIN6wbDXItl95euhFz2uON45H2qjYt+0=
 github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A=
 github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo=
-github.com/jesseduffield/generics v0.0.0-20220320043834-727e535cbe68 h1:EQP2Tv8TIcC6Y4RI+1ZbJDOHfGJ570tPeYVCqo7/tws=
-github.com/jesseduffield/generics v0.0.0-20220320043834-727e535cbe68/go.mod h1:+LLj9/WUPAP8LqCchs7P+7X0R98HiFujVFANdNaxhGk=
+github.com/jesseduffield/generics v0.0.0-20250406224309-4f541cb84918 h1:meoUDZGF6jZAbhW5IBwj92mTqGmrOn+Cuu0jM7/aUcs=
+github.com/jesseduffield/generics v0.0.0-20250406224309-4f541cb84918/go.mod h1:+LLj9/WUPAP8LqCchs7P+7X0R98HiFujVFANdNaxhGk=
 github.com/jesseduffield/go-git/v5 v5.1.2-0.20221018185014-fdd53fef665d h1:bO+OmbreIv91rCe8NmscRwhFSqkDJtzWCPV4Y+SQuXE=
 github.com/jesseduffield/go-git/v5 v5.1.2-0.20221018185014-fdd53fef665d/go.mod h1:nGNEErzf+NRznT+N2SWqmHnDnF9aLgANB1CUNEan09o=
 github.com/jesseduffield/gocui v0.3.1-0.20250220081214-b376cb0857ac h1:vUNTiVEB9Bz16pTJ5kNgb/1HhnWdSA1P0GfFLUJeITI=

pkg/gui/controllers/files_controller.go

@@ -6,6 +6,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/jesseduffield/generics/set"
 	"github.com/jesseduffield/gocui"
 	"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
@@ -1194,13 +1195,36 @@ func filterNodesHaveUnstagedChanges(nodes []*filetree.FileNode) []*filetree.File
 	})
 }
 
+func findSubmoduleNode(nodes []*filetree.FileNode, submodules []*models.SubmoduleConfig) *models.File {
+	for _, node := range nodes {
+		submoduleNode := node.FindFirstFileBy(func(f *models.File) bool {
+			return f.IsSubmodule(submodules)
+		})
+		if submoduleNode != nil {
+			return submoduleNode
+		}
+	}
+	return nil
+}
+
 func (self *FilesController) canRemove(selectedNodes []*filetree.FileNode) *types.DisabledReason {
+	// Return disabled if the selection contains multiple changed items and includes a submodule change.
 	submodules := self.c.Model().Submodules
-	submoduleCount := lo.CountBy(selectedNodes, func(node *filetree.FileNode) bool {
-		return node.File != nil && node.File.IsSubmodule(submodules)
-	})
-	if submoduleCount > 0 && len(selectedNodes) > 1 {
-		return &types.DisabledReason{Text: self.c.Tr.RangeSelectNotSupportedForSubmodules}
+	hasFiles := false
+	uniqueSelectedSubmodules := set.New[*models.SubmoduleConfig]()
+
+	for _, node := range selectedNodes {
+		_ = node.ForEachFile(func(f *models.File) error {
+			if submodule := f.SubmoduleConfig(submodules); submodule != nil {
+				uniqueSelectedSubmodules.Add(submodule)
+			} else {
+				hasFiles = true
+			}
+			return nil
+		})
+		if uniqueSelectedSubmodules.Len() > 0 && (hasFiles || uniqueSelectedSubmodules.Len() > 1) {
+			return &types.DisabledReason{Text: self.c.Tr.MultiSelectNotSupportedForSubmodules}
+		}
 	}
 
 	return nil
@@ -1209,11 +1233,13 @@ func (self *FilesController) canRemove(selectedNodes []*filetree.FileNode) *type
 func (self *FilesController) remove(selectedNodes []*filetree.FileNode) error {
 	submodules := self.c.Model().Submodules
 
+	selectedNodes = normalisedSelectedNodes(selectedNodes)
+
 	// If we have one submodule then we must only have one submodule or `canRemove` would have
 	// returned an error
-	firstNode := selectedNodes[0]
-	if firstNode.File != nil && firstNode.File.IsSubmodule(submodules) {
-		submodule := firstNode.File.SubmoduleConfig(submodules)
+	submoduleNode := findSubmoduleNode(selectedNodes, submodules)
+	if submoduleNode != nil {
+		submodule := submoduleNode.SubmoduleConfig(submodules)
 
 		menuItems := []*types.MenuItem{
 			{
@@ -1224,11 +1250,9 @@ func (self *FilesController) remove(selectedNodes []*filetree.FileNode) error {
 			},
 		}
 
-		return self.c.Menu(types.CreateMenuOptions{Title: firstNode.GetPath(), Items: menuItems})
+		return self.c.Menu(types.CreateMenuOptions{Title: submoduleNode.GetPath(), Items: menuItems})
 	}
 
-	selectedNodes = normalisedSelectedNodes(selectedNodes)
-
 	discardAllChangesItem := types.MenuItem{
 		Label: self.c.Tr.DiscardAllChanges,
 		OnPress: func() error {

pkg/gui/filetree/node.go

@@ -109,13 +109,13 @@ func (self *Node[T]) SortChildren() {
 	self.Children = children
 }
 
-func (self *Node[T]) Some(test func(*Node[T]) bool) bool {
-	if test(self) {
+func (self *Node[T]) Some(predicate func(*Node[T]) bool) bool {
+	if predicate(self) {
 		return true
 	}
 
 	for _, child := range self.Children {
-		if child.Some(test) {
+		if child.Some(predicate) {
 			return true
 		}
 	}
@@ -123,14 +123,14 @@ func (self *Node[T]) Some(test func(*Node[T]) bool) bool {
 	return false
 }
 
-func (self *Node[T]) SomeFile(test func(*T) bool) bool {
+func (self *Node[T]) SomeFile(predicate func(*T) bool) bool {
 	if self.IsFile() {
-		if test(self.File) {
+		if predicate(self.File) {
 			return true
 		}
 	} else {
 		for _, child := range self.Children {
-			if child.SomeFile(test) {
+			if child.SomeFile(predicate) {
 				return true
 			}
 		}
@@ -139,13 +139,13 @@ func (self *Node[T]) SomeFile(test func(*T) bool) bool {
 	return false
 }
 
-func (self *Node[T]) Every(test func(*Node[T]) bool) bool {
-	if !test(self) {
+func (self *Node[T]) Every(predicate func(*Node[T]) bool) bool {
+	if !predicate(self) {
 		return false
 	}
 
 	for _, child := range self.Children {
-		if !child.Every(test) {
+		if !child.Every(predicate) {
 			return false
 		}
 	}
@@ -153,14 +153,14 @@ func (self *Node[T]) Every(test func(*Node[T]) bool) bool {
 	return true
 }
 
-func (self *Node[T]) EveryFile(test func(*T) bool) bool {
+func (self *Node[T]) EveryFile(predicate func(*T) bool) bool {
 	if self.IsFile() {
-		if !test(self.File) {
+		if !predicate(self.File) {
 			return false
 		}
 	} else {
 		for _, child := range self.Children {
-			if !child.EveryFile(test) {
+			if !child.EveryFile(predicate) {
 				return false
 			}
 		}
@@ -169,6 +169,22 @@ func (self *Node[T]) EveryFile(test func(*T) bool) bool {
 	return true
 }
 
+func (self *Node[T]) FindFirstFileBy(predicate func(*T) bool) *T {
+	if self.IsFile() {
+		if predicate(self.File) {
+			return self.File
+		}
+	} else {
+		for _, child := range self.Children {
+			if file := child.FindFirstFileBy(predicate); file != nil {
+				return file
+			}
+		}
+	}
+
+	return nil
+}
+
 func (self *Node[T]) Flatten(collapsedPaths *CollapsedPaths) []*Node[T] {
 	result := []*Node[T]{self}
 
@@ -279,23 +295,23 @@ func (self *Node[T]) compressAux() *Node[T] {
 	return self
 }
 
-func (self *Node[T]) GetPathsMatching(test func(*Node[T]) bool) []string {
+func (self *Node[T]) GetPathsMatching(predicate func(*Node[T]) bool) []string {
 	paths := []string{}
 
-	if test(self) {
+	if predicate(self) {
 		paths = append(paths, self.GetPath())
 	}
 
 	for _, child := range self.Children {
-		paths = append(paths, child.GetPathsMatching(test)...)
+		paths = append(paths, child.GetPathsMatching(predicate)...)
 	}
 
 	return paths
 }
 
-func (self *Node[T]) GetFilePathsMatching(test func(*T) bool) []string {
+func (self *Node[T]) GetFilePathsMatching(predicate func(*T) bool) []string {
 	matchingFileNodes := lo.Filter(self.GetLeaves(), func(node *Node[T], _ int) bool {
-		return test(node.File)
+		return predicate(node.File)
 	})
 
 	return lo.Map(matchingFileNodes, func(node *Node[T], _ int) string {

pkg/i18n/english.go

@@ -845,7 +845,7 @@ type TranslationSet struct {
 	NoItemSelected                           string
 	SelectedItemIsNotABranch                 string
 	SelectedItemDoesNotHaveFiles             string
-	RangeSelectNotSupportedForSubmodules     string
+	MultiSelectNotSupportedForSubmodules     string
 	OldCherryPickKeyWarning                  string
 	CommandDoesNotSupportOpeningInEditor     string
 	CustomCommands                           string
@@ -1889,7 +1889,7 @@ func EnglishTranslationSet() *TranslationSet {
 		NoItemSelected:                           "No item selected",
 		SelectedItemIsNotABranch:                 "Selected item is not a branch",
 		SelectedItemDoesNotHaveFiles:             "Selected item does not have files to view",
-		RangeSelectNotSupportedForSubmodules:     "Range select not supported for submodules",
+		MultiSelectNotSupportedForSubmodules:     "Multiselection not supported for submodules",
 		OldCherryPickKeyWarning:                  "The 'c' key is no longer the default key for copying commits to cherry pick. Please use `{{.copy}}` instead (and `{{.paste}}` to paste). The reason for this change is that the 'v' key for selecting a range of lines when staging is now also used for selecting a range of lines in any list view, meaning that we needed to find a new key for pasting commits, and if we're going to now use `{{.paste}}` for pasting commits, we may as well use `{{.copy}}` for copying them. If you want to configure the keybindings to get the old behaviour, set the following in your config:\n\nkeybinding:\n  universal:\n    toggleRangeSelect: <something other than v>\n  commits:\n    cherryPickCopy: 'c'\n    pasteCommits: 'v'",
 		CommandDoesNotSupportOpeningInEditor:     "This command doesn't support switching to the editor",
 		CustomCommands:                           "Custom commands",

pkg/integration/components/shell.go

@@ -360,8 +360,8 @@ func (self *Shell) CloneIntoRemote(name string) *Shell {
 }
 
 func (self *Shell) CloneIntoSubmodule(submoduleName string, submodulePath string) *Shell {
-	self.Clone("other_repo")
-	self.RunCommand([]string{"git", "submodule", "add", "--name", submoduleName, "../other_repo", submodulePath})
+	self.Clone(submoduleName)
+	self.RunCommand([]string{"git", "submodule", "add", "--name", submoduleName, "../" + submoduleName, submodulePath})
 
 	return self
 }

pkg/integration/tests/submodule/remove.go

@@ -44,7 +44,7 @@ var Remove = NewIntegrationTest(NewIntegrationTestArgs{
 		t.Views().Main().Content(
 			Contains("-[submodule \"my_submodule_name\"]").
 				Contains("-   path = my_submodule_path").
-				Contains("-   url = ../other_repo"),
+				Contains("-   url = ../my_submodule_name"),
 		)
 
 		t.FileSystem().PathNotPresent(gitDirSubmodulePath)

pkg/integration/tests/submodule/reset.go

@@ -75,7 +75,7 @@ var Reset = NewIntegrationTest(NewIntegrationTestArgs{
 				Equals("   M my_submodule_path (submodule)"),
 				Equals("  ?? other_file").IsSelected(),
 			).
-			// Verify we can't use range select on submodules
+			// Verify we can't reset a submodule and file change at the same time.
 			Press(keys.Universal.ToggleRangeSelect).
 			SelectPreviousItem().
 			Lines(
@@ -85,7 +85,7 @@ var Reset = NewIntegrationTest(NewIntegrationTestArgs{
 			).
 			Press(keys.Universal.Remove).
 			Tap(func() {
-				t.ExpectToast(Contains("Disabled: Range select not supported for submodules"))
+				t.ExpectToast(Contains("Disabled: Multiselection not supported for submodules"))
 			}).
 			Press(keys.Universal.ToggleRangeSelect).
 			Lines(

pkg/integration/tests/submodule/reset_folder.go

@@ -0,0 +1,126 @@
+package submodule
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var ResetFolder = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Reset submodule changes located in a nested folder.",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig:  func(cfg *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.EmptyCommit("first commit")
+		shell.CreateDir("dir")
+		shell.CloneIntoSubmodule("submodule1", "dir/submodule1")
+		shell.CloneIntoSubmodule("submodule2", "dir/submodule2")
+		shell.GitAddAll()
+		shell.Commit("add submodules")
+
+		shell.CreateFile("dir/submodule1/file", "")
+		shell.CreateFile("dir/submodule2/file", "")
+		shell.CreateFile("dir/file", "")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Files().Focus().
+			Lines(
+				Equals("▼ dir").IsSelected(),
+				Equals("  ?? file"),
+				Equals("   M submodule1 (submodule)"),
+				Equals("   M submodule2 (submodule)"),
+			).
+			// Verify we cannot reset the entire folder (has nested file and submodule changes).
+			Press(keys.Universal.Remove).
+			Tap(func() {
+				t.ExpectToast(Contains("Disabled: Multiselection not supported for submodules"))
+			}).
+			// Verify we cannot reset submodule + file or submodule + submodule via range select.
+			SelectNextItem().
+			Press(keys.Universal.ToggleRangeSelect).
+			SelectNextItem().
+			Lines(
+				Equals("▼ dir"),
+				Equals("  ?? file").IsSelected(),
+				Equals("   M submodule1 (submodule)").IsSelected(),
+				Equals("   M submodule2 (submodule)"),
+			).
+			Press(keys.Universal.Remove).
+			Tap(func() {
+				t.ExpectToast(Contains("Disabled: Multiselection not supported for submodules"))
+			}).
+			Press(keys.Universal.ToggleRangeSelect).
+			Press(keys.Universal.ToggleRangeSelect).
+			SelectNextItem().
+			Lines(
+				Equals("▼ dir"),
+				Equals("  ?? file"),
+				Equals("   M submodule1 (submodule)").IsSelected(),
+				Equals("   M submodule2 (submodule)").IsSelected(),
+			).
+			Press(keys.Universal.Remove).
+			Tap(func() {
+				t.ExpectToast(Contains("Disabled: Multiselection not supported for submodules"))
+			}).
+			// Reset the file change.
+			Press(keys.Universal.ToggleRangeSelect).
+			NavigateToLine(Contains("file")).
+			Press(keys.Universal.Remove).
+			Tap(func() {
+				t.ExpectPopup().Menu().
+					Title(Equals("Discard changes")).
+					Select(Contains("Discard all changes")).
+					Confirm()
+			}).
+			NavigateToLine(Contains("▼ dir")).
+			Lines(
+				Equals("▼ dir").IsSelected(),
+				Equals("   M submodule1 (submodule)"),
+				Equals("   M submodule2 (submodule)"),
+			).
+			// Verify we still cannot reset the entire folder (has two submodule changes).
+			Press(keys.Universal.Remove).
+			Tap(func() {
+				t.ExpectToast(Contains("Disabled: Multiselection not supported for submodules"))
+			}).
+			// Reset one of the submodule changes.
+			SelectNextItem().
+			Press(keys.Universal.Remove).
+			Tap(func() {
+				t.ExpectPopup().Menu().
+					Title(Equals("dir/submodule1")).
+					Select(Contains("Stash uncommitted submodule changes and update")).
+					Confirm()
+			}).
+			NavigateToLine(Contains("▼ dir")).
+			Lines(
+				Equals("▼ dir").IsSelected(),
+				Equals("   M submodule2 (submodule)"),
+			).
+			// Now we can reset the folder (equivalent to resetting just the nested submodule change).
+			// Range selecting both the folder and submodule change is allowed.
+			Press(keys.Universal.ToggleRangeSelect).
+			SelectNextItem().
+			Lines(
+				Equals("▼ dir").IsSelected(),
+				Equals("   M submodule2 (submodule)").IsSelected(),
+			).
+			Press(keys.Universal.Remove).
+			Tap(func() {
+				t.ExpectPopup().Menu().
+					Title(Equals("dir/submodule2")).
+					Select(Contains("Stash uncommitted submodule changes and update")).
+					Cancel()
+			}).
+			// Or just selecting the folder itself.
+			NavigateToLine(Contains("▼ dir")).
+			Press(keys.Universal.Remove).
+			Tap(func() {
+				t.ExpectPopup().Menu().
+					Title(Equals("dir/submodule2")).
+					Select(Contains("Stash uncommitted submodule changes and update")).
+					Confirm()
+			}).
+			IsEmpty()
+	},
+})

pkg/integration/tests/test_list.go

@@ -345,6 +345,7 @@ var tests = []*components.IntegrationTest{
 	submodule.Remove,
 	submodule.RemoveNested,
 	submodule.Reset,
+	submodule.ResetFolder,
 	sync.FetchPrune,
 	sync.FetchWhenSortedByDate,
 	sync.ForcePush,

vendor/github.com/jesseduffield/generics/set/set.go

@@ -39,6 +39,10 @@ func (s *Set[T]) Includes(value T) bool {
 	return s.hashMap[value]
 }
 
+func (s *Set[T]) Len() int {
+	return len(s.hashMap)
+}
+
 // output slice is not necessarily in the same order that items were added
 func (s *Set[T]) ToSlice() []T {
 	return maps.Keys(s.hashMap)

vendor/modules.txt

@@ -120,7 +120,7 @@ github.com/integrii/flaggy
 # github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99
 ## explicit
 github.com/jbenet/go-context/io
-# github.com/jesseduffield/generics v0.0.0-20220320043834-727e535cbe68
+# github.com/jesseduffield/generics v0.0.0-20250406224309-4f541cb84918
 ## explicit; go 1.18
 github.com/jesseduffield/generics/maps
 github.com/jesseduffield/generics/set