From a75b0457c18fae987e320d389d8e4f17466185e8 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 12 Oct 2026 18:48:13 +0200 Subject: [PATCH] Ask before discarding a nested repository if that loses anything Discarding a nested repository deletes its directory like that of any untracked file. If the repository keeps its git dir in its directory, this deletes all of its history, and nothing said so. The directory of a removed submodule keeps its git dir elsewhere, but it may have changes that aren't committed. Check for both before discarding. If anything would be lost, list it and ask for confirmation. Co-Authored-By: Claude Opus 5.5 (1M context) --- pkg/commands/git_commands/working_tree.go | 13 +++ pkg/gui/controllers/files_controller.go | 98 +++++++++++++++---- pkg/i18n/english.go | 8 ++ .../tests/file/discard_nested_repositories.go | 69 +++++++++++++ pkg/integration/tests/test_list.go | 1 + 5 files changed, 169 insertions(+), 20 deletions(-) create mode 100644 pkg/integration/tests/file/discard_nested_repositories.go diff --git a/pkg/commands/git_commands/working_tree.go b/pkg/commands/git_commands/working_tree.go index 09c86a882..db68d694f 100644 --- a/pkg/commands/git_commands/working_tree.go +++ b/pkg/commands/git_commands/working_tree.go @@ -433,6 +433,19 @@ func (self *WorkingTreeCommands) NestedRepositoryHasOwnGitDir(path string) bool return err == nil && info.IsDir() } +// NestedRepositoryHasUncommittedChanges reports whether the git repository at +// the given path of the working tree has changes that aren't committed, +// including untracked files +func (self *WorkingTreeCommands) NestedRepositoryHasUncommittedChanges(path string) (bool, error) { + cmdArgs := NewGitCmd("status"). + Dir(path). + Arg("--porcelain", "--untracked-files=normal"). + ToArgv() + + output, err := ForOtherRepo(self.cmd.New(cmdArgs)).DontLog().RunWithOutput() + return strings.TrimSpace(output) != "", err +} + // UntrackedFilesCmdObj returns the command that lists the untracked files in // the given directory, for the main view func (self *WorkingTreeCommands) UntrackedFilesCmdObj(dirPath string) *oscommands.CmdObj { diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index 3d2bd686c..c696f0406 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -1879,19 +1879,21 @@ func (self *FilesController) remove(selectedNodes []*filetree.FileNode) error { discardAllChangesItem := types.MenuItem{ Label: self.c.Tr.DiscardAllChanges, OnPress: func() error { - self.c.LogAction(self.c.Tr.Actions.DiscardAllChangesInFile) + return self.confirmDiscardingNestedRepositories(selectedNodes, func() error { + self.c.LogAction(self.c.Tr.Actions.DiscardAllChangesInFile) - if self.context().IsSelectingRange() { - defer self.context().CancelRangeSelect() - } + if self.context().IsSelectingRange() { + defer self.context().CancelRangeSelect() + } - nodes := lo.Map(selectedNodes, func(n *filetree.FileNode, _ int) git_commands.IFileNode { return n }) - if err := self.c.Git().WorkingTree.DiscardAllDirChanges(nodes); err != nil { - return err - } + nodes := lo.Map(selectedNodes, func(n *filetree.FileNode, _ int) git_commands.IFileNode { return n }) + if err := self.c.Git().WorkingTree.DiscardAllDirChanges(nodes); err != nil { + return err + } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.WORKTREES}}) - return nil + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.WORKTREES}}) + return nil + }) }, Keys: self.c.KeybindingsOpts().GetKeys(self.c.UserConfig().Keybinding.Files.ConfirmDiscard), Tooltip: utils.ResolvePlaceholderString( @@ -1905,19 +1907,21 @@ func (self *FilesController) remove(selectedNodes []*filetree.FileNode) error { discardUnstagedChangesItem := types.MenuItem{ Label: self.c.Tr.DiscardUnstagedChanges, OnPress: func() error { - self.c.LogAction(self.c.Tr.Actions.DiscardAllUnstagedChangesInFile) + return self.confirmDiscardingNestedRepositories(selectedNodes, func() error { + self.c.LogAction(self.c.Tr.Actions.DiscardAllUnstagedChangesInFile) - if self.context().IsSelectingRange() { - defer self.context().CancelRangeSelect() - } + if self.context().IsSelectingRange() { + defer self.context().CancelRangeSelect() + } - nodes := lo.Map(selectedNodes, func(n *filetree.FileNode, _ int) git_commands.IFileNode { return n }) - if err := self.c.Git().WorkingTree.DiscardUnstagedDirChanges(nodes); err != nil { - return err - } + nodes := lo.Map(selectedNodes, func(n *filetree.FileNode, _ int) git_commands.IFileNode { return n }) + if err := self.c.Git().WorkingTree.DiscardUnstagedDirChanges(nodes); err != nil { + return err + } - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.WORKTREES}}) - return nil + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.WORKTREES}}) + return nil + }) }, Keys: menuKey('u'), Tooltip: utils.ResolvePlaceholderString( @@ -1940,6 +1944,60 @@ func (self *FilesController) remove(selectedNodes []*filetree.FileNode) error { return self.c.Menu(types.CreateMenuOptions{Title: self.c.Tr.DiscardChangesTitle, Items: menuItems}) } +// confirmDiscardingNestedRepositories calls discard, but asks first if +// discarding the given nodes would discard something of a nested repository in +// them that can't be restored: the repository itself, if it keeps its git dir +// inside its directory, or changes that it has that aren't committed. +func (self *FilesController) confirmDiscardingNestedRepositories(nodes []*filetree.FileNode, discard func() error) error { + var nestedRepositories []*models.File + for _, node := range nodes { + _ = node.ForEachFile(func(file *models.File) error { + if file.IsNestedRepository() { + nestedRepositories = append(nestedRepositories, file) + } + return nil + }) + } + + if len(nestedRepositories) == 0 { + return discard() + } + + return self.c.WithWaitingStatus(self.c.Tr.CheckingNestedRepositories, func(gocui.Task) error { + var losses []string + for _, file := range nestedRepositories { + placeholders := map[string]string{"path": file.Path} + if self.c.Git().WorkingTree.NestedRepositoryHasOwnGitDir(file.Path) { + losses = append(losses, utils.ResolvePlaceholderString( + self.c.Tr.NestedRepositoryHistoryWouldBeLost, placeholders)) + continue + } + + // If we can't tell, assume that there are changes, so that we ask + hasChanges, err := self.c.Git().WorkingTree.NestedRepositoryHasUncommittedChanges(file.Path) + if hasChanges || err != nil { + losses = append(losses, utils.ResolvePlaceholderString( + self.c.Tr.NestedRepositoryChangesWouldBeLost, placeholders)) + } + } + + self.c.OnUIThread(func() error { + if len(losses) == 0 { + return discard() + } + + self.c.Confirm(types.ConfirmOpts{ + Title: self.c.Tr.DiscardChangesTitle, + Prompt: self.c.Tr.DiscardNestedRepositoriesPrompt + "\n\n- " + strings.Join(losses, "\n- "), + HandleConfirm: discard, + }) + return nil + }) + + return nil + }) +} + func (self *FilesController) ResetSubmodule(submodule *models.SubmoduleConfig) error { file := self.c.Helpers().WorkingTree.FileForSubmodule(submodule) return self.c.WithWaitingStatus(self.c.Tr.ResettingSubmoduleStatus, func(gocui.Task) error { diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 4f3110624..b71d084a6 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -117,6 +117,10 @@ type TranslationSet struct { PlainDirectoryFiles string StageNestedRepositoryDisabled string StageWorktreeDisabled string + CheckingNestedRepositories string + DiscardNestedRepositoriesPrompt string + NestedRepositoryHistoryWouldBeLost string + NestedRepositoryChangesWouldBeLost string StageConflictsRangeDisabled string Checkout string CheckoutTooltip string @@ -1295,6 +1299,10 @@ func EnglishTranslationSet() *TranslationSet { NestedRepositoryStatus: "Status of '{{.path}}'", StageNestedRepositoryDisabled: "Can't stage '{{.path}}' because it is a git repository of its own. Discard it with {{.discardKey}}, or ignore it with {{.ignoreKey}}.", StageWorktreeDisabled: "Can't stage '{{.path}}' because it is a worktree. Remove it with {{.removeKey}}, or ignore it with {{.ignoreKey}}.", + CheckingNestedRepositories: "Checking for uncommitted changes", + DiscardNestedRepositoriesPrompt: "Discarding this also discards the following. Are you sure?", + NestedRepositoryHistoryWouldBeLost: "'{{.path}}': the repository with all of its history", + NestedRepositoryChangesWouldBeLost: "'{{.path}}': uncommitted changes", PlainDirectoryDescription: "'{{.path}}' is an untracked directory. Git shows it as a single entry because status.showUntrackedFiles is set to \"normal\".", PlainDirectoryFiles: "Files in '{{.path}}'", StageConflictsRangeDisabled: "Cannot stage a selection that includes files with merge conflicts; resolve them individually with {{.goIntoKey}} first.", diff --git a/pkg/integration/tests/file/discard_nested_repositories.go b/pkg/integration/tests/file/discard_nested_repositories.go new file mode 100644 index 000000000..b286a263f --- /dev/null +++ b/pkg/integration/tests/file/discard_nested_repositories.go @@ -0,0 +1,69 @@ +package file + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var DiscardNestedRepositories = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Discarding a nested repository asks first if that would discard its history or changes that it has that aren't committed", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Gui.ShowFileTree = false + }, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("first commit") + shell.CloneIntoSubmodule("sub", "modules/sub") + shell.Commit("add submodule") + shell.RunCommand([]string{"git", "rm", "--cached", "modules/sub"}) + shell.RunCommand([]string{"git", "rm", ".gitmodules"}) + shell.Commit("remove submodule") + shell.CreateFile("modules/sub/new-file", "content") + + shell.RunCommand([]string{"git", "init", "nested"}) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Lines( + Equals("?? modules/sub (removed submodule)").IsSelected(), + Equals("?? nested (nested repository)"), + ). + Press(keys.Universal.RangeSelectDown). + Press(keys.Universal.Remove). + Tap(func() { + t.ExpectPopup().Menu(). + Title(Equals("Discard changes")). + Select(Contains("Discard all changes")). + Confirm() + + t.ExpectPopup().Confirmation(). + Title(Equals("Discard changes")). + Content(Equals("Discarding this also discards the following. Are you sure?\n\n" + + "- 'modules/sub': uncommitted changes\n" + + "- 'nested': the repository with all of its history")). + Cancel() + }). + Lines( + Equals("?? modules/sub (removed submodule)").IsSelected(), + Equals("?? nested (nested repository)").IsSelected(), + ). + Press(keys.Universal.Remove). + Tap(func() { + t.ExpectPopup().Menu(). + Title(Equals("Discard changes")). + Select(Contains("Discard all changes")). + Confirm() + + t.ExpectPopup().Confirmation(). + Title(Equals("Discard changes")). + Content(Contains("Are you sure?")). + Confirm() + }). + IsEmpty() + + t.FileSystem().PathNotPresent("modules/sub") + t.FileSystem().PathNotPresent("nested") + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 0736fecf6..36b71ea00 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -245,6 +245,7 @@ var tests = []*components.IntegrationTest{ file.DirectoryDiffWithRenamedFiles, file.DiscardAllDirChanges, file.DiscardAllDirChangesWhenFiltering, + file.DiscardNestedRepositories, file.DiscardRangeSelect, file.DiscardStagedChanges, file.DiscardUnstagedDirChanges,