Skip to content

Commit 959f932

Browse files
stefanhallerstefanhaller
authored
Fix discarding submodule changes in nested folders (#4317)
- **PR Description** This PR addresses #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.
2 parents facc73a + f75c0af commit 959f932

File tree

12 files changed

+210
-39
lines changed

12 files changed

+210
-39
lines changed

‎go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ require (
1313
github.com/gookit/color v1.4.2
1414
github.com/imdario/mergo v0.3.11
1515
github.com/integrii/flaggy v1.4.0
16-
github.com/jesseduffield/generics v0.0.0-20220320043834-727e535cbe68
16+
github.com/jesseduffield/generics v0.0.0-20250406224309-4f541cb84918
1717
github.com/jesseduffield/go-git/v5 v5.1.2-0.20221018185014-fdd53fef665d
1818
github.com/jesseduffield/gocui v0.3.1-0.20250220081214-b376cb0857ac
1919
github.com/jesseduffield/kill v0.0.0-20250101124109-e216ddbe133a

‎go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,8 @@ github.com/invopop/jsonschema v0.10.0 h1:c1ktzNLBun3LyQQhyty5WE3lulbOdIIyOVlkmDL
182182
github.com/invopop/jsonschema v0.10.0/go.mod h1:ffZ5Km5SWWRAIN6wbDXItl95euhFz2uON45H2qjYt+0=
183183
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOlocH6Fxy8MmwDt+yVQYULKfN0RoTN8A=
184184
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo=
185-
github.com/jesseduffield/generics v0.0.0-20220320043834-727e535cbe68 h1:EQP2Tv8TIcC6Y4RI+1ZbJDOHfGJ570tPeYVCqo7/tws=
186-
github.com/jesseduffield/generics v0.0.0-20220320043834-727e535cbe68/go.mod h1:+LLj9/WUPAP8LqCchs7P+7X0R98HiFujVFANdNaxhGk=
185+
github.com/jesseduffield/generics v0.0.0-20250406224309-4f541cb84918 h1:meoUDZGF6jZAbhW5IBwj92mTqGmrOn+Cuu0jM7/aUcs=
186+
github.com/jesseduffield/generics v0.0.0-20250406224309-4f541cb84918/go.mod h1:+LLj9/WUPAP8LqCchs7P+7X0R98HiFujVFANdNaxhGk=
187187
github.com/jesseduffield/go-git/v5 v5.1.2-0.20221018185014-fdd53fef665d h1:bO+OmbreIv91rCe8NmscRwhFSqkDJtzWCPV4Y+SQuXE=
188188
github.com/jesseduffield/go-git/v5 v5.1.2-0.20221018185014-fdd53fef665d/go.mod h1:nGNEErzf+NRznT+N2SWqmHnDnF9aLgANB1CUNEan09o=
189189
github.com/jesseduffield/gocui v0.3.1-0.20250220081214-b376cb0857ac h1:vUNTiVEB9Bz16pTJ5kNgb/1HhnWdSA1P0GfFLUJeITI=

‎pkg/gui/controllers/files_controller.go

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"path/filepath"
77
"strings"
88

9+
"github.com/jesseduffield/generics/set"
910
"github.com/jesseduffield/gocui"
1011
"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
1112
"github.com/jesseduffield/lazygit/pkg/commands/models"
@@ -1194,13 +1195,36 @@ func filterNodesHaveUnstagedChanges(nodes []*filetree.FileNode) []*filetree.File
11941195
})
11951196
}
11961197

1198+
func findSubmoduleNode(nodes []*filetree.FileNode, submodules []*models.SubmoduleConfig) *models.File {
1199+
for _, node := range nodes {
1200+
submoduleNode := node.FindFirstFileBy(func(f *models.File) bool {
1201+
return f.IsSubmodule(submodules)
1202+
})
1203+
if submoduleNode != nil {
1204+
return submoduleNode
1205+
}
1206+
}
1207+
return nil
1208+
}
1209+
11971210
func (self *FilesController) canRemove(selectedNodes []*filetree.FileNode) *types.DisabledReason {
1211+
// Return disabled if the selection contains multiple changed items and includes a submodule change.
11981212
submodules := self.c.Model().Submodules
1199-
submoduleCount := lo.CountBy(selectedNodes, func(node *filetree.FileNode) bool {
1200-
return node.File != nil && node.File.IsSubmodule(submodules)
1201-
})
1202-
if submoduleCount > 0 && len(selectedNodes) > 1 {
1203-
return &types.DisabledReason{Text: self.c.Tr.RangeSelectNotSupportedForSubmodules}
1213+
hasFiles := false
1214+
uniqueSelectedSubmodules := set.New[*models.SubmoduleConfig]()
1215+
1216+
for _, node := range selectedNodes {
1217+
_ = node.ForEachFile(func(f *models.File) error {
1218+
if submodule := f.SubmoduleConfig(submodules); submodule != nil {
1219+
uniqueSelectedSubmodules.Add(submodule)
1220+
} else {
1221+
hasFiles = true
1222+
}
1223+
return nil
1224+
})
1225+
if uniqueSelectedSubmodules.Len() > 0 && (hasFiles || uniqueSelectedSubmodules.Len() > 1) {
1226+
return &types.DisabledReason{Text: self.c.Tr.MultiSelectNotSupportedForSubmodules}
1227+
}
12041228
}
12051229

12061230
return nil
@@ -1209,11 +1233,13 @@ func (self *FilesController) canRemove(selectedNodes []*filetree.FileNode) *type
12091233
func (self *FilesController) remove(selectedNodes []*filetree.FileNode) error {
12101234
submodules := self.c.Model().Submodules
12111235

1236+
selectedNodes = normalisedSelectedNodes(selectedNodes)
1237+
12121238
// If we have one submodule then we must only have one submodule or `canRemove` would have
12131239
// returned an error
1214-
firstNode := selectedNodes[0]
1215-
if firstNode.File != nil && firstNode.File.IsSubmodule(submodules) {
1216-
submodule := firstNode.File.SubmoduleConfig(submodules)
1240+
submoduleNode := findSubmoduleNode(selectedNodes, submodules)
1241+
if submoduleNode != nil {
1242+
submodule := submoduleNode.SubmoduleConfig(submodules)
12171243

12181244
menuItems := []*types.MenuItem{
12191245
{
@@ -1224,11 +1250,9 @@ func (self *FilesController) remove(selectedNodes []*filetree.FileNode) error {
12241250
},
12251251
}
12261252

1227-
return self.c.Menu(types.CreateMenuOptions{Title: firstNode.GetPath(), Items: menuItems})
1253+
return self.c.Menu(types.CreateMenuOptions{Title: submoduleNode.GetPath(), Items: menuItems})
12281254
}
12291255

1230-
selectedNodes = normalisedSelectedNodes(selectedNodes)
1231-
12321256
discardAllChangesItem := types.MenuItem{
12331257
Label: self.c.Tr.DiscardAllChanges,
12341258
OnPress: func() error {

‎pkg/gui/filetree/node.go

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -109,28 +109,28 @@ func (self *Node[T]) SortChildren() {
109109
self.Children = children
110110
}
111111

112-
func (self *Node[T]) Some(test func(*Node[T]) bool) bool {
113-
if test(self) {
112+
func (self *Node[T]) Some(predicate func(*Node[T]) bool) bool {
113+
if predicate(self) {
114114
return true
115115
}
116116

117117
for _, child := range self.Children {
118-
if child.Some(test) {
118+
if child.Some(predicate) {
119119
return true
120120
}
121121
}
122122

123123
return false
124124
}
125125

126-
func (self *Node[T]) SomeFile(test func(*T) bool) bool {
126+
func (self *Node[T]) SomeFile(predicate func(*T) bool) bool {
127127
if self.IsFile() {
128-
if test(self.File) {
128+
if predicate(self.File) {
129129
return true
130130
}
131131
} else {
132132
for _, child := range self.Children {
133-
if child.SomeFile(test) {
133+
if child.SomeFile(predicate) {
134134
return true
135135
}
136136
}
@@ -139,28 +139,28 @@ func (self *Node[T]) SomeFile(test func(*T) bool) bool {
139139
return false
140140
}
141141

142-
func (self *Node[T]) Every(test func(*Node[T]) bool) bool {
143-
if !test(self) {
142+
func (self *Node[T]) Every(predicate func(*Node[T]) bool) bool {
143+
if !predicate(self) {
144144
return false
145145
}
146146

147147
for _, child := range self.Children {
148-
if !child.Every(test) {
148+
if !child.Every(predicate) {
149149
return false
150150
}
151151
}
152152

153153
return true
154154
}
155155

156-
func (self *Node[T]) EveryFile(test func(*T) bool) bool {
156+
func (self *Node[T]) EveryFile(predicate func(*T) bool) bool {
157157
if self.IsFile() {
158-
if !test(self.File) {
158+
if !predicate(self.File) {
159159
return false
160160
}
161161
} else {
162162
for _, child := range self.Children {
163-
if !child.EveryFile(test) {
163+
if !child.EveryFile(predicate) {
164164
return false
165165
}
166166
}
@@ -169,6 +169,22 @@ func (self *Node[T]) EveryFile(test func(*T) bool) bool {
169169
return true
170170
}
171171

172+
func (self *Node[T]) FindFirstFileBy(predicate func(*T) bool) *T {
173+
if self.IsFile() {
174+
if predicate(self.File) {
175+
return self.File
176+
}
177+
} else {
178+
for _, child := range self.Children {
179+
if file := child.FindFirstFileBy(predicate); file != nil {
180+
return file
181+
}
182+
}
183+
}
184+
185+
return nil
186+
}
187+
172188
func (self *Node[T]) Flatten(collapsedPaths *CollapsedPaths) []*Node[T] {
173189
result := []*Node[T]{self}
174190

@@ -279,23 +295,23 @@ func (self *Node[T]) compressAux() *Node[T] {
279295
return self
280296
}
281297

282-
func (self *Node[T]) GetPathsMatching(test func(*Node[T]) bool) []string {
298+
func (self *Node[T]) GetPathsMatching(predicate func(*Node[T]) bool) []string {
283299
paths := []string{}
284300

285-
if test(self) {
301+
if predicate(self) {
286302
paths = append(paths, self.GetPath())
287303
}
288304

289305
for _, child := range self.Children {
290-
paths = append(paths, child.GetPathsMatching(test)...)
306+
paths = append(paths, child.GetPathsMatching(predicate)...)
291307
}
292308

293309
return paths
294310
}
295311

296-
func (self *Node[T]) GetFilePathsMatching(test func(*T) bool) []string {
312+
func (self *Node[T]) GetFilePathsMatching(predicate func(*T) bool) []string {
297313
matchingFileNodes := lo.Filter(self.GetLeaves(), func(node *Node[T], _ int) bool {
298-
return test(node.File)
314+
return predicate(node.File)
299315
})
300316

301317
return lo.Map(matchingFileNodes, func(node *Node[T], _ int) string {

‎pkg/i18n/english.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -845,7 +845,7 @@ type TranslationSet struct {
845845
NoItemSelected string
846846
SelectedItemIsNotABranch string
847847
SelectedItemDoesNotHaveFiles string
848-
RangeSelectNotSupportedForSubmodules string
848+
MultiSelectNotSupportedForSubmodules string
849849
OldCherryPickKeyWarning string
850850
CommandDoesNotSupportOpeningInEditor string
851851
CustomCommands string
@@ -1889,7 +1889,7 @@ func EnglishTranslationSet() *TranslationSet {
18891889
NoItemSelected: "No item selected",
18901890
SelectedItemIsNotABranch: "Selected item is not a branch",
18911891
SelectedItemDoesNotHaveFiles: "Selected item does not have files to view",
1892-
RangeSelectNotSupportedForSubmodules: "Range select not supported for submodules",
1892+
MultiSelectNotSupportedForSubmodules: "Multiselection not supported for submodules",
18931893
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'",
18941894
CommandDoesNotSupportOpeningInEditor: "This command doesn't support switching to the editor",
18951895
CustomCommands: "Custom commands",

‎pkg/integration/components/shell.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,8 +360,8 @@ func (self *Shell) CloneIntoRemote(name string) *Shell {
360360
}
361361

362362
func (self *Shell) CloneIntoSubmodule(submoduleName string, submodulePath string) *Shell {
363-
self.Clone("other_repo")
364-
self.RunCommand([]string{"git", "submodule", "add", "--name", submoduleName, "../other_repo", submodulePath})
363+
self.Clone(submoduleName)
364+
self.RunCommand([]string{"git", "submodule", "add", "--name", submoduleName, "../" + submoduleName, submodulePath})
365365

366366
return self
367367
}

‎pkg/integration/tests/submodule/remove.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ var Remove = NewIntegrationTest(NewIntegrationTestArgs{
4444
t.Views().Main().Content(
4545
Contains("-[submodule \"my_submodule_name\"]").
4646
Contains("- path = my_submodule_path").
47-
Contains("- url = ../other_repo"),
47+
Contains("- url = ../my_submodule_name"),
4848
)
4949

5050
t.FileSystem().PathNotPresent(gitDirSubmodulePath)

‎pkg/integration/tests/submodule/reset.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ var Reset = NewIntegrationTest(NewIntegrationTestArgs{
7575
Equals(" M my_submodule_path (submodule)"),
7676
Equals(" ?? other_file").IsSelected(),
7777
).
78-
// Verify we can't use range select on submodules
78+
// Verify we can't reset a submodule and file change at the same time.
7979
Press(keys.Universal.ToggleRangeSelect).
8080
SelectPreviousItem().
8181
Lines(
@@ -85,7 +85,7 @@ var Reset = NewIntegrationTest(NewIntegrationTestArgs{
8585
).
8686
Press(keys.Universal.Remove).
8787
Tap(func() {
88-
t.ExpectToast(Contains("Disabled: Range select not supported for submodules"))
88+
t.ExpectToast(Contains("Disabled: Multiselection not supported for submodules"))
8989
}).
9090
Press(keys.Universal.ToggleRangeSelect).
9191
Lines(

0 commit comments

Comments
 (0)