From 85a6a42bff05c38d0485ff40ce064145604c661b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 10 Feb 2024 19:42:30 +0100 Subject: [PATCH 1/5] Extend submodule tests to use a submodule where name and path are different In most real-world scenarios, name and path are usually the same. They don't have to be though, and it's important to make sure we use the right one when passing arguments to git commands, so change the tests to have different name and path. --- pkg/integration/components/git.go | 11 +++++++++++ pkg/integration/components/shell.go | 4 ++-- pkg/integration/tests/submodule/enter.go | 12 ++++++++---- pkg/integration/tests/submodule/remove.go | 12 ++++++------ pkg/integration/tests/submodule/reset.go | 22 ++++++++++++---------- 5 files changed, 39 insertions(+), 22 deletions(-) diff --git a/pkg/integration/components/git.go b/pkg/integration/components/git.go index ed327b3ed..1e2975be2 100644 --- a/pkg/integration/components/git.go +++ b/pkg/integration/components/git.go @@ -2,7 +2,10 @@ package components import ( "fmt" + "log" "strings" + + "github.com/jesseduffield/lazygit/pkg/commands/git_commands" ) type Git struct { @@ -44,3 +47,11 @@ func (self *Git) expect(cmdArgs []string, condition func(string) (bool, string)) return self } + +func (self *Git) Version() *git_commands.GitVersion { + version, err := getGitVersion() + if err != nil { + log.Fatalf("Could not get git version: %v", err) + } + return version +} diff --git a/pkg/integration/components/shell.go b/pkg/integration/components/shell.go index 48ff3fdf7..60c627918 100644 --- a/pkg/integration/components/shell.go +++ b/pkg/integration/components/shell.go @@ -345,9 +345,9 @@ func (self *Shell) CloneIntoRemote(name string) *Shell { return self } -func (self *Shell) CloneIntoSubmodule(submoduleName string) *Shell { +func (self *Shell) CloneIntoSubmodule(submoduleName string, submodulePath string) *Shell { self.Clone("other_repo") - self.RunCommand([]string{"git", "submodule", "add", "../other_repo", submoduleName}) + self.RunCommand([]string{"git", "submodule", "add", "--name", submoduleName, "../other_repo", submodulePath}) return self } diff --git a/pkg/integration/tests/submodule/enter.go b/pkg/integration/tests/submodule/enter.go index 7b055c2b6..29e983b7f 100644 --- a/pkg/integration/tests/submodule/enter.go +++ b/pkg/integration/tests/submodule/enter.go @@ -20,7 +20,7 @@ var Enter = NewIntegrationTest(NewIntegrationTestArgs{ }, SetupRepo: func(shell *Shell) { shell.EmptyCommit("first commit") - shell.CloneIntoSubmodule("my_submodule") + shell.CloneIntoSubmodule("my_submodule_name", "my_submodule_path") shell.GitAddAll() shell.Commit("add submodule") }, @@ -29,14 +29,18 @@ var Enter = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Status().Content(Contains("repo")) } assertInSubmodule := func() { - t.Views().Status().Content(Contains("my_submodule")) + if t.Git().Version().IsAtLeast(2, 22, 0) { + t.Views().Status().Content(Contains("my_submodule_path(my_submodule_name)")) + } else { + t.Views().Status().Content(Contains("my_submodule_path")) + } } assertInParentRepo() t.Views().Submodules().Focus(). Lines( - Contains("my_submodule").IsSelected(), + Contains("my_submodule_name").IsSelected(), ). // enter the submodule PressEnter() @@ -60,7 +64,7 @@ var Enter = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Files().Focus(). Lines( - MatchesRegexp(` M.*my_submodule \(submodule\)`).IsSelected(), + MatchesRegexp(` M.*my_submodule_path \(submodule\)`).IsSelected(), ). Tap(func() { // main view also shows the new commit when we're looking at the submodule within the files view diff --git a/pkg/integration/tests/submodule/remove.go b/pkg/integration/tests/submodule/remove.go index 3d85d4d5c..886eb6294 100644 --- a/pkg/integration/tests/submodule/remove.go +++ b/pkg/integration/tests/submodule/remove.go @@ -12,20 +12,20 @@ var Remove = NewIntegrationTest(NewIntegrationTestArgs{ SetupConfig: func(config *config.AppConfig) {}, SetupRepo: func(shell *Shell) { shell.EmptyCommit("first commit") - shell.CloneIntoSubmodule("my_submodule") + shell.CloneIntoSubmodule("my_submodule_name", "my_submodule_path") shell.GitAddAll() shell.Commit("add submodule") }, Run: func(t *TestDriver, keys config.KeybindingConfig) { t.Views().Submodules().Focus(). Lines( - Contains("my_submodule").IsSelected(), + Contains("my_submodule_name").IsSelected(), ). Press(keys.Universal.Remove). Tap(func() { t.ExpectPopup().Confirmation(). Title(Equals("Remove submodule")). - Content(Equals("Are you sure you want to remove submodule 'my_submodule' and its corresponding directory? This is irreversible.")). + Content(Equals("Are you sure you want to remove submodule 'my_submodule_name' and its corresponding directory? This is irreversible.")). Confirm() }). IsEmpty() @@ -33,12 +33,12 @@ var Remove = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Files().Focus(). Lines( MatchesRegexp(`M.*\.gitmodules`).IsSelected(), - MatchesRegexp(`D.*my_submodule`), + MatchesRegexp(`D.*my_submodule_path`), ) t.Views().Main().Content( - Contains("-[submodule \"my_submodule\"]"). - Contains("- path = my_submodule"). + Contains("-[submodule \"my_submodule_name\"]"). + Contains("- path = my_submodule_path"). Contains("- url = ../other_repo"), ) }, diff --git a/pkg/integration/tests/submodule/reset.go b/pkg/integration/tests/submodule/reset.go index fba91bee8..a723561fc 100644 --- a/pkg/integration/tests/submodule/reset.go +++ b/pkg/integration/tests/submodule/reset.go @@ -20,7 +20,7 @@ var Reset = NewIntegrationTest(NewIntegrationTestArgs{ }, SetupRepo: func(shell *Shell) { shell.EmptyCommit("first commit") - shell.CloneIntoSubmodule("my_submodule") + shell.CloneIntoSubmodule("my_submodule_name", "my_submodule_path") shell.GitAddAll() shell.Commit("add submodule") @@ -31,22 +31,24 @@ var Reset = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Status().Content(Contains("repo")) } assertInSubmodule := func() { - t.Views().Status().Content(Contains("my_submodule")) + if t.Git().Version().IsAtLeast(2, 22, 0) { + t.Views().Status().Content(Contains("my_submodule_path(my_submodule_name)")) + } else { + t.Views().Status().Content(Contains("my_submodule_path")) + } } assertInParentRepo() t.Views().Submodules().Focus(). Lines( - Contains("my_submodule").IsSelected(), + Contains("my_submodule_name").IsSelected(), ). // enter the submodule PressEnter() assertInSubmodule() - t.Views().Status().Content(Contains("my_submodule")) - t.Views().Files().IsFocused(). Press("e"). Tap(func() { @@ -65,18 +67,18 @@ var Reset = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Submodules().IsFocused() - t.Views().Main().Content(Contains("Submodule my_submodule contains modified content")) + t.Views().Main().Content(Contains("Submodule my_submodule_path contains modified content")) t.Views().Files().Focus(). Lines( - MatchesRegexp(` M.*my_submodule \(submodule\)`), + MatchesRegexp(` M.*my_submodule_path \(submodule\)`), Contains("other_file").IsSelected(), ). // Verify we can't use range select on submodules Press(keys.Universal.ToggleRangeSelect). SelectPreviousItem(). Lines( - MatchesRegexp(` M.*my_submodule \(submodule\)`).IsSelected(), + MatchesRegexp(` M.*my_submodule_path \(submodule\)`).IsSelected(), Contains("other_file").IsSelected(), ). Press(keys.Universal.Remove). @@ -85,13 +87,13 @@ var Reset = NewIntegrationTest(NewIntegrationTestArgs{ }). Press(keys.Universal.ToggleRangeSelect). Lines( - MatchesRegexp(` M.*my_submodule \(submodule\)`).IsSelected(), + MatchesRegexp(` M.*my_submodule_path \(submodule\)`).IsSelected(), Contains("other_file"), ). Press(keys.Universal.Remove). Tap(func() { t.ExpectPopup().Menu(). - Title(Equals("my_submodule")). + Title(Equals("my_submodule_path")). Select(Contains("Stash uncommitted submodule changes and update")). Confirm() }). From ddcd9163018e264321046023d49ab176f71733c7 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 10 Feb 2024 19:44:43 +0100 Subject: [PATCH 2/5] Add test to check that the git dir for a deleted submodule was removed The test shows that it actually doesn't work when the submodule name is different from its path. We'll fix this in the next commit. --- pkg/integration/tests/submodule/remove.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/integration/tests/submodule/remove.go b/pkg/integration/tests/submodule/remove.go index 886eb6294..f4f1cd04f 100644 --- a/pkg/integration/tests/submodule/remove.go +++ b/pkg/integration/tests/submodule/remove.go @@ -17,6 +17,9 @@ var Remove = NewIntegrationTest(NewIntegrationTestArgs{ shell.Commit("add submodule") }, Run: func(t *TestDriver, keys config.KeybindingConfig) { + gitDirSubmodulePath := ".git/modules/my_submodule_name" + t.FileSystem().PathPresent(gitDirSubmodulePath) + t.Views().Submodules().Focus(). Lines( Contains("my_submodule_name").IsSelected(), @@ -41,5 +44,10 @@ var Remove = NewIntegrationTest(NewIntegrationTestArgs{ Contains("- path = my_submodule_path"). Contains("- url = ../other_repo"), ) + + /* EXPECTED: + t.FileSystem().PathNotPresent(gitDirSubmodulePath) + ACTUAL: */ + t.FileSystem().PathPresent(gitDirSubmodulePath) }, }) From ea87912a740464d1c69f4acad9c79efe6588d532 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 10 Feb 2024 19:47:36 +0100 Subject: [PATCH 3/5] Fix deleting submodule where name and path are different --- pkg/commands/git_commands/submodule.go | 2 +- pkg/integration/tests/submodule/remove.go | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/commands/git_commands/submodule.go b/pkg/commands/git_commands/submodule.go index d9d1ccd20..3f69ce875 100644 --- a/pkg/commands/git_commands/submodule.go +++ b/pkg/commands/git_commands/submodule.go @@ -141,7 +141,7 @@ func (self *SubmoduleCommands) Delete(submodule *models.SubmoduleConfig) error { // We may in fact want to use the repo's git dir path but git docs say not to // mix submodules and worktrees anyway. - return os.RemoveAll(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "modules", submodule.Path)) + return os.RemoveAll(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "modules", submodule.Name)) } func (self *SubmoduleCommands) Add(name string, path string, url string) error { diff --git a/pkg/integration/tests/submodule/remove.go b/pkg/integration/tests/submodule/remove.go index f4f1cd04f..22fb83f30 100644 --- a/pkg/integration/tests/submodule/remove.go +++ b/pkg/integration/tests/submodule/remove.go @@ -45,9 +45,6 @@ var Remove = NewIntegrationTest(NewIntegrationTestArgs{ Contains("- url = ../other_repo"), ) - /* EXPECTED: t.FileSystem().PathNotPresent(gitDirSubmodulePath) - ACTUAL: */ - t.FileSystem().PathPresent(gitDirSubmodulePath) }, }) From db4f12929ec242182e482ee5ab829d463cda6ea5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 10 Feb 2024 18:07:42 +0100 Subject: [PATCH 4/5] Pass entire submodule to UpdateUrl instead of name and path separately This will make the next commit slightly simpler. --- pkg/commands/git_commands/submodule.go | 6 +++--- pkg/gui/controllers/submodules_controller.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/commands/git_commands/submodule.go b/pkg/commands/git_commands/submodule.go index 3f69ce875..b105d57ea 100644 --- a/pkg/commands/git_commands/submodule.go +++ b/pkg/commands/git_commands/submodule.go @@ -158,10 +158,10 @@ func (self *SubmoduleCommands) Add(name string, path string, url string) error { return self.cmd.New(cmdArgs).Run() } -func (self *SubmoduleCommands) UpdateUrl(name string, path string, newUrl string) error { +func (self *SubmoduleCommands) UpdateUrl(submodule *models.SubmoduleConfig, newUrl string) error { setUrlCmdStr := NewGitCmd("config"). Arg( - "--file", ".gitmodules", "submodule."+name+".url", newUrl, + "--file", ".gitmodules", "submodule."+submodule.Name+".url", newUrl, ). ToArgv() @@ -170,7 +170,7 @@ func (self *SubmoduleCommands) UpdateUrl(name string, path string, newUrl string return err } - syncCmdStr := NewGitCmd("submodule").Arg("sync", "--", path). + syncCmdStr := NewGitCmd("submodule").Arg("sync", "--", submodule.Path). ToArgv() if err := self.cmd.New(syncCmdStr).Run(); err != nil { diff --git a/pkg/gui/controllers/submodules_controller.go b/pkg/gui/controllers/submodules_controller.go index 3cf3b5bf5..13496ce73 100644 --- a/pkg/gui/controllers/submodules_controller.go +++ b/pkg/gui/controllers/submodules_controller.go @@ -183,7 +183,7 @@ func (self *SubmodulesController) editURL(submodule *models.SubmoduleConfig) err HandleConfirm: func(newUrl string) error { return self.c.WithWaitingStatus(self.c.Tr.UpdatingSubmoduleUrlStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.UpdateSubmoduleUrl) - err := self.c.Git().Submodule.UpdateUrl(submodule.Name, submodule.Path, newUrl) + err := self.c.Git().Submodule.UpdateUrl(submodule, newUrl) if err != nil { _ = self.c.Error(err) } From 3b723282cbe98063523e22f9dd71000d20dc5e20 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 4 Feb 2024 22:42:58 +0100 Subject: [PATCH 5/5] Show all submodules recursively --- .../git_commands/git_command_builder.go | 8 +++ pkg/commands/git_commands/submodule.go | 65 +++++++++++++++---- pkg/commands/git_commands/working_tree.go | 2 +- pkg/commands/models/submodule_config.go | 31 ++++++++- pkg/gui/context/submodules_context.go | 2 +- pkg/gui/controllers/helpers/refresh_helper.go | 2 +- pkg/gui/controllers/helpers/repos_helper.go | 2 +- pkg/gui/controllers/submodules_controller.go | 8 +-- pkg/gui/presentation/submodules.go | 12 +++- .../tests/submodule/enter_nested.go | 52 +++++++++++++++ .../tests/submodule/remove_nested.go | 56 ++++++++++++++++ pkg/integration/tests/submodule/shared.go | 39 +++++++++++ pkg/integration/tests/test_list.go | 2 + 13 files changed, 260 insertions(+), 21 deletions(-) create mode 100644 pkg/integration/tests/submodule/enter_nested.go create mode 100644 pkg/integration/tests/submodule/remove_nested.go create mode 100644 pkg/integration/tests/submodule/shared.go diff --git a/pkg/commands/git_commands/git_command_builder.go b/pkg/commands/git_commands/git_command_builder.go index 4aa35be5f..b6fe57364 100644 --- a/pkg/commands/git_commands/git_command_builder.go +++ b/pkg/commands/git_commands/git_command_builder.go @@ -60,6 +60,14 @@ func (self *GitCommandBuilder) Dir(path string) *GitCommandBuilder { return self } +func (self *GitCommandBuilder) DirIf(condition bool, path string) *GitCommandBuilder { + if condition { + return self.Dir(path) + } + + return self +} + // Note, you may prefer to use the Dir method instead of this one func (self *GitCommandBuilder) Worktree(path string) *GitCommandBuilder { // worktree arg comes before the command diff --git a/pkg/commands/git_commands/submodule.go b/pkg/commands/git_commands/submodule.go index b105d57ea..40a0d3509 100644 --- a/pkg/commands/git_commands/submodule.go +++ b/pkg/commands/git_commands/submodule.go @@ -26,8 +26,12 @@ func NewSubmoduleCommands(gitCommon *GitCommon) *SubmoduleCommands { } } -func (self *SubmoduleCommands) GetConfigs() ([]*models.SubmoduleConfig, error) { - file, err := os.Open(".gitmodules") +func (self *SubmoduleCommands) GetConfigs(parentModule *models.SubmoduleConfig) ([]*models.SubmoduleConfig, error) { + gitModulesPath := ".gitmodules" + if parentModule != nil { + gitModulesPath = filepath.Join(parentModule.FullPath(), gitModulesPath) + } + file, err := os.Open(gitModulesPath) if err != nil { if os.IsNotExist(err) { return nil, nil @@ -51,21 +55,27 @@ func (self *SubmoduleCommands) GetConfigs() ([]*models.SubmoduleConfig, error) { } configs := []*models.SubmoduleConfig{} + lastConfigIdx := -1 for scanner.Scan() { line := scanner.Text() if name, ok := firstMatch(line, `\[submodule "(.*)"\]`); ok { - configs = append(configs, &models.SubmoduleConfig{Name: name}) + configs = append(configs, &models.SubmoduleConfig{ + Name: name, ParentModule: parentModule, + }) + lastConfigIdx = len(configs) - 1 continue } - if len(configs) > 0 { - lastConfig := configs[len(configs)-1] - + if lastConfigIdx != -1 { if path, ok := firstMatch(line, `\s*path\s*=\s*(.*)\s*`); ok { - lastConfig.Path = path + configs[lastConfigIdx].Path = path + nestedConfigs, err := self.GetConfigs(configs[lastConfigIdx]) + if err == nil { + configs = append(configs, nestedConfigs...) + } } else if url, ok := firstMatch(line, `\s*url\s*=\s*(.*)\s*`); ok { - lastConfig.Url = url + configs[lastConfigIdx].Url = url } } } @@ -77,12 +87,12 @@ func (self *SubmoduleCommands) Stash(submodule *models.SubmoduleConfig) error { // if the path does not exist then it hasn't yet been initialized so we'll swallow the error // because the intention here is to have no dirty worktree state if _, err := os.Stat(submodule.Path); os.IsNotExist(err) { - self.Log.Infof("submodule path %s does not exist, returning", submodule.Path) + self.Log.Infof("submodule path %s does not exist, returning", submodule.FullPath()) return nil } cmdArgs := NewGitCmd("stash"). - Dir(submodule.Path). + Dir(submodule.FullPath()). Arg("--include-untracked"). ToArgv() @@ -90,8 +100,13 @@ func (self *SubmoduleCommands) Stash(submodule *models.SubmoduleConfig) error { } func (self *SubmoduleCommands) Reset(submodule *models.SubmoduleConfig) error { + parentDir := "" + if submodule.ParentModule != nil { + parentDir = submodule.ParentModule.FullPath() + } cmdArgs := NewGitCmd("submodule"). Arg("update", "--init", "--force", "--", submodule.Path). + DirIf(parentDir != "", parentDir). ToArgv() return self.cmd.New(cmdArgs).Run() @@ -107,6 +122,20 @@ func (self *SubmoduleCommands) UpdateAll() error { func (self *SubmoduleCommands) Delete(submodule *models.SubmoduleConfig) error { // based on https://gist.github.com/myusuf3/7f645819ded92bda6677 + if submodule.ParentModule != nil { + wd, err := os.Getwd() + if err != nil { + return err + } + + err = os.Chdir(submodule.ParentModule.FullPath()) + if err != nil { + return err + } + + defer func() { _ = os.Chdir(wd) }() + } + if err := self.cmd.New( NewGitCmd("submodule"). Arg("deinit", "--force", "--", submodule.Path).ToArgv(), @@ -141,7 +170,7 @@ func (self *SubmoduleCommands) Delete(submodule *models.SubmoduleConfig) error { // We may in fact want to use the repo's git dir path but git docs say not to // mix submodules and worktrees anyway. - return os.RemoveAll(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "modules", submodule.Name)) + return os.RemoveAll(submodule.GitDirPath(self.repoPaths.repoGitDirPath)) } func (self *SubmoduleCommands) Add(name string, path string, url string) error { @@ -159,6 +188,20 @@ func (self *SubmoduleCommands) Add(name string, path string, url string) error { } func (self *SubmoduleCommands) UpdateUrl(submodule *models.SubmoduleConfig, newUrl string) error { + if submodule.ParentModule != nil { + wd, err := os.Getwd() + if err != nil { + return err + } + + err = os.Chdir(submodule.ParentModule.FullPath()) + if err != nil { + return err + } + + defer func() { _ = os.Chdir(wd) }() + } + setUrlCmdStr := NewGitCmd("config"). Arg( "--file", ".gitmodules", "submodule."+submodule.Name+".url", newUrl, diff --git a/pkg/commands/git_commands/working_tree.go b/pkg/commands/git_commands/working_tree.go index 2bb82578d..99665d7cc 100644 --- a/pkg/commands/git_commands/working_tree.go +++ b/pkg/commands/git_commands/working_tree.go @@ -343,7 +343,7 @@ func (self *WorkingTreeCommands) RemoveUntrackedFiles() error { // ResetAndClean removes all unstaged changes and removes all untracked files func (self *WorkingTreeCommands) ResetAndClean() error { - submoduleConfigs, err := self.submodule.GetConfigs() + submoduleConfigs, err := self.submodule.GetConfigs(nil) if err != nil { return err } diff --git a/pkg/commands/models/submodule_config.go b/pkg/commands/models/submodule_config.go index f52576921..7df0d131a 100644 --- a/pkg/commands/models/submodule_config.go +++ b/pkg/commands/models/submodule_config.go @@ -1,13 +1,33 @@ package models +import "path/filepath" + type SubmoduleConfig struct { Name string Path string Url string + + ParentModule *SubmoduleConfig // nil if top-level +} + +func (r *SubmoduleConfig) FullName() string { + if r.ParentModule != nil { + return r.ParentModule.FullName() + "/" + r.Name + } + + return r.Name +} + +func (r *SubmoduleConfig) FullPath() string { + if r.ParentModule != nil { + return r.ParentModule.FullPath() + "/" + r.Path + } + + return r.Path } func (r *SubmoduleConfig) RefName() string { - return r.Name + return r.FullName() } func (r *SubmoduleConfig) ID() string { @@ -17,3 +37,12 @@ func (r *SubmoduleConfig) ID() string { func (r *SubmoduleConfig) Description() string { return r.RefName() } + +func (r *SubmoduleConfig) GitDirPath(repoGitDirPath string) string { + parentPath := repoGitDirPath + if r.ParentModule != nil { + parentPath = r.ParentModule.GitDirPath(repoGitDirPath) + } + + return filepath.Join(parentPath, "modules", r.Name) +} diff --git a/pkg/gui/context/submodules_context.go b/pkg/gui/context/submodules_context.go index aff8f64ab..dbd12077a 100644 --- a/pkg/gui/context/submodules_context.go +++ b/pkg/gui/context/submodules_context.go @@ -17,7 +17,7 @@ func NewSubmodulesContext(c *ContextCommon) *SubmodulesContext { viewModel := NewFilteredListViewModel( func() []*models.SubmoduleConfig { return c.Model().Submodules }, func(submodule *models.SubmoduleConfig) []string { - return []string{submodule.Name} + return []string{submodule.FullName()} }, nil, ) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index da43c47bb..fd0d11881 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -415,7 +415,7 @@ func (self *RefreshHelper) refreshTags() error { } func (self *RefreshHelper) refreshStateSubmoduleConfigs() error { - configs, err := self.c.Git().Submodule.GetConfigs() + configs, err := self.c.Git().Submodule.GetConfigs(nil) if err != nil { return err } diff --git a/pkg/gui/controllers/helpers/repos_helper.go b/pkg/gui/controllers/helpers/repos_helper.go index 59d45e0c1..c4a00cb73 100644 --- a/pkg/gui/controllers/helpers/repos_helper.go +++ b/pkg/gui/controllers/helpers/repos_helper.go @@ -48,7 +48,7 @@ func (self *ReposHelper) EnterSubmodule(submodule *models.SubmoduleConfig) error } self.c.State().GetRepoPathStack().Push(wd) - return self.DispatchSwitchToRepo(submodule.Path, context.NO_CONTEXT) + return self.DispatchSwitchToRepo(submodule.FullPath(), context.NO_CONTEXT) } func (self *ReposHelper) getCurrentBranch(path string) string { diff --git a/pkg/gui/controllers/submodules_controller.go b/pkg/gui/controllers/submodules_controller.go index 13496ce73..dde1a1f46 100644 --- a/pkg/gui/controllers/submodules_controller.go +++ b/pkg/gui/controllers/submodules_controller.go @@ -116,8 +116,8 @@ func (self *SubmodulesController) GetOnRenderToMain() func() error { } else { prefix := fmt.Sprintf( "Name: %s\nPath: %s\nUrl: %s\n\n", - style.FgGreen.Sprint(submodule.Name), - style.FgYellow.Sprint(submodule.Path), + style.FgGreen.Sprint(submodule.FullName()), + style.FgYellow.Sprint(submodule.FullPath()), style.FgCyan.Sprint(submodule.Url), ) @@ -178,7 +178,7 @@ func (self *SubmodulesController) add() error { func (self *SubmodulesController) editURL(submodule *models.SubmoduleConfig) error { return self.c.Prompt(types.PromptOpts{ - Title: fmt.Sprintf(self.c.Tr.UpdateSubmoduleUrl, submodule.Name), + Title: fmt.Sprintf(self.c.Tr.UpdateSubmoduleUrl, submodule.FullName()), InitialContent: submodule.Url, HandleConfirm: func(newUrl string) error { return self.c.WithWaitingStatus(self.c.Tr.UpdatingSubmoduleUrlStatus, func(gocui.Task) error { @@ -272,7 +272,7 @@ func (self *SubmodulesController) update(submodule *models.SubmoduleConfig) erro func (self *SubmodulesController) remove(submodule *models.SubmoduleConfig) error { return self.c.Confirm(types.ConfirmOpts{ Title: self.c.Tr.RemoveSubmodule, - Prompt: fmt.Sprintf(self.c.Tr.RemoveSubmodulePrompt, submodule.Name), + Prompt: fmt.Sprintf(self.c.Tr.RemoveSubmodulePrompt, submodule.FullName()), HandleConfirm: func() error { self.c.LogAction(self.c.Tr.Actions.RemoveSubmodule) if err := self.c.Git().Submodule.Delete(submodule); err != nil { diff --git a/pkg/gui/presentation/submodules.go b/pkg/gui/presentation/submodules.go index e580ee1f6..72c6bfc08 100644 --- a/pkg/gui/presentation/submodules.go +++ b/pkg/gui/presentation/submodules.go @@ -13,5 +13,15 @@ func GetSubmoduleListDisplayStrings(submodules []*models.SubmoduleConfig) [][]st } func getSubmoduleDisplayStrings(s *models.SubmoduleConfig) []string { - return []string{theme.DefaultTextColor.Sprint(s.Name)} + name := s.Name + if s.ParentModule != nil { + indentation := "" + for p := s.ParentModule; p != nil; p = p.ParentModule { + indentation += " " + } + + name = indentation + "- " + s.Name + } + + return []string{theme.DefaultTextColor.Sprint(name)} } diff --git a/pkg/integration/tests/submodule/enter_nested.go b/pkg/integration/tests/submodule/enter_nested.go new file mode 100644 index 000000000..172dfbfae --- /dev/null +++ b/pkg/integration/tests/submodule/enter_nested.go @@ -0,0 +1,52 @@ +package submodule + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var EnterNested = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Enter a nested submodule", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(cfg *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + setupNestedSubmodules(shell) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Submodules().Focus(). + Lines( + Equals("outerSubName").IsSelected(), + Equals(" - innerSubName"), + ). + Tap(func() { + t.Views().Main().ContainsLines( + Contains("Name: outerSubName"), + Contains("Path: modules/outerSubPath"), + Contains("Url: ../outerSubmodule"), + ) + }). + SelectNextItem(). + Tap(func() { + t.Views().Main().ContainsLines( + Contains("Name: outerSubName/innerSubName"), + Contains("Path: modules/outerSubPath/modules/innerSubPath"), + Contains("Url: ../innerSubmodule"), + ) + }). + // enter the nested submodule + PressEnter() + + if t.Git().Version().IsAtLeast(2, 22, 0) { + t.Views().Status().Content(Contains("innerSubPath(innerSubName)")) + } else { + t.Views().Status().Content(Contains("innerSubPath")) + } + t.Views().Commits().ContainsLines( + Contains("initial inner commit"), + ) + + t.Views().Files().PressEscape() + t.Views().Status().Content(Contains("repo")) + }, +}) diff --git a/pkg/integration/tests/submodule/remove_nested.go b/pkg/integration/tests/submodule/remove_nested.go new file mode 100644 index 000000000..ae32c0907 --- /dev/null +++ b/pkg/integration/tests/submodule/remove_nested.go @@ -0,0 +1,56 @@ +package submodule + +import ( + "path/filepath" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var RemoveNested = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Remove a nested submodule", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + setupNestedSubmodules(shell) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + gitDirSubmodulePath, _ := filepath.Abs(".git/modules/outerSubName/modules/innerSubName") + t.FileSystem().PathPresent(gitDirSubmodulePath) + + t.Views().Submodules().Focus(). + Lines( + Equals("outerSubName").IsSelected(), + Equals(" - innerSubName"), + ). + SelectNextItem(). + Press(keys.Universal.Remove). + Tap(func() { + t.ExpectPopup().Confirmation(). + Title(Equals("Remove submodule")). + Content(Equals("Are you sure you want to remove submodule 'outerSubName/innerSubName' and its corresponding directory? This is irreversible.")). + Confirm() + }). + Lines( + Equals("outerSubName").IsSelected(), + ). + Press(keys.Universal.GoInto) + + t.Views().Files().IsFocused(). + Lines( + Contains("modules").IsSelected(), + MatchesRegexp(`D.*innerSubPath`), + MatchesRegexp(`M.*\.gitmodules`), + ). + NavigateToLine(Contains(".gitmodules")) + + t.Views().Main().Content( + Contains("-[submodule \"innerSubName\"]"). + Contains("- path = modules/innerSubPath"). + Contains("- url = ../innerSubmodule"), + ) + + t.FileSystem().PathNotPresent(gitDirSubmodulePath) + }, +}) diff --git a/pkg/integration/tests/submodule/shared.go b/pkg/integration/tests/submodule/shared.go new file mode 100644 index 000000000..43e0144ab --- /dev/null +++ b/pkg/integration/tests/submodule/shared.go @@ -0,0 +1,39 @@ +package submodule + +import ( + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +func setupNestedSubmodules(shell *Shell) { + // we're going to have a directory structure like this: + // project + // - repo/modules/outerSubName/modules/innerSubName/ + // + shell.CreateFileAndAdd("rootFile", "rootStuff") + shell.Commit("initial repo commit") + + shell.Chdir("..") + shell.CreateDir("innerSubmodule") + shell.Chdir("innerSubmodule") + shell.Init() + shell.CreateFileAndAdd("inner", "inner") + shell.Commit("initial inner commit") + + shell.Chdir("..") + shell.CreateDir("outerSubmodule") + shell.Chdir("outerSubmodule") + shell.Init() + shell.CreateFileAndAdd("outer", "outer") + shell.Commit("initial outer commit") + shell.CreateDir("modules") + // the git config (-c) parameter below is required + // to let git create a file-protocol/path submodule + shell.RunCommand([]string{"git", "-c", "protocol.file.allow=always", "submodule", "add", "--name", "innerSubName", "../innerSubmodule", "modules/innerSubPath"}) + shell.Commit("add dependency as innerSubmodule") + + shell.Chdir("../repo") + shell.CreateDir("modules") + shell.RunCommand([]string{"git", "-c", "protocol.file.allow=always", "submodule", "add", "--name", "outerSubName", "../outerSubmodule", "modules/outerSubPath"}) + shell.Commit("add dependency as outerSubmodule") + shell.RunCommand([]string{"git", "-c", "protocol.file.allow=always", "submodule", "update", "--init", "--recursive"}) +} diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index e26a0731f..531fce5d9 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -245,7 +245,9 @@ var tests = []*components.IntegrationTest{ stash.StashUnstaged, submodule.Add, submodule.Enter, + submodule.EnterNested, submodule.Remove, + submodule.RemoveNested, submodule.Reset, sync.FetchPrune, sync.FetchWhenSortedByDate,