From 3f0a7512f8ca251f17db757da52ced7d344f00ae Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 2 Jun 2026 12:09:09 +0200 Subject: [PATCH] Fix unstaging a submodule with dirty content The stage/unstage toggle decides what to do based on whether a node has unstaged changes: if it does, it stages; otherwise it unstages. For a submodule this breaks down, because dirty or untracked content inside the submodule always reports as an unstaged change in the parent repo but can never be staged from there. Once such a submodule's commit pointer is staged it sits at "MM", and every subsequent press keeps trying to stage the unstageable dirty content, so it can never be unstaged. Treat a submodule's unstaged change as stageable only when its commit isn't already staged, so that a staged submodule unstages on the next press regardless of leftover dirty content. Because the decision is now shared by press and stage-all, this fixes both at once. Co-Authored-By: Claude Opus 4.8 (1M context) --- pkg/gui/controllers/files_controller.go | 26 +++++++++++++++++++++--- pkg/integration/tests/submodule/stage.go | 5 ----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/pkg/gui/controllers/files_controller.go b/pkg/gui/controllers/files_controller.go index c03ff71ab..d8f883e40 100644 --- a/pkg/gui/controllers/files_controller.go +++ b/pkg/gui/controllers/files_controller.go @@ -471,7 +471,7 @@ func (self *FilesController) toggleStaged( nodes = normalisedSelectedNodes(nodes) - unstagedNodes := filterNodesHaveUnstagedChanges(nodes) + unstagedNodes := filterNodesHaveUnstagedChanges(nodes, self.c.Model().Submodules) if len(unstagedNodes) > 0 { self.c.LogAction(stageAction) @@ -1421,12 +1421,32 @@ func someNodesHaveStagedChanges(nodes []*filetree.FileNode) bool { return lo.SomeBy(nodes, (*filetree.FileNode).GetHasStagedChanges) } -func filterNodesHaveUnstagedChanges(nodes []*filetree.FileNode) []*filetree.FileNode { +func filterNodesHaveUnstagedChanges(nodes []*filetree.FileNode, submodules []*models.SubmoduleConfig) []*filetree.FileNode { return lo.Filter(nodes, func(node *filetree.FileNode, _ int) bool { - return node.GetHasUnstagedChanges() + return node.SomeFile(func(file *models.File) bool { + return fileHasStageableUnstagedChanges(file, submodules) + }) }) } +// For a submodule, the only thing the parent repo can stage is the +// commit-pointer change; dirty or untracked content within the submodule +// shows up as an unstaged change but can never be staged from the parent. So +// once the submodule's commit is staged (leaving it at e.g. "MM"), we mustn't +// treat the leftover unstaged change as stageable, or pressing space would +// keep trying to stage it instead of unstaging it. +func fileHasStageableUnstagedChanges(file *models.File, submodules []*models.SubmoduleConfig) bool { + if !file.HasUnstagedChanges { + return false + } + + if file.IsSubmodule(submodules) { + return !file.HasStagedChanges + } + + return true +} + func findSubmoduleNode(nodes []*filetree.FileNode, submodules []*models.SubmoduleConfig) *models.File { for _, node := range nodes { submoduleNode := node.FindFirstFileBy(func(f *models.File) bool { diff --git a/pkg/integration/tests/submodule/stage.go b/pkg/integration/tests/submodule/stage.go index 7303a0dbc..324d7690f 100644 --- a/pkg/integration/tests/submodule/stage.go +++ b/pkg/integration/tests/submodule/stage.go @@ -39,13 +39,8 @@ var Stage = NewIntegrationTest(NewIntegrationTestArgs{ // Pressing again must unstage the submodule, taking us back to // " M" rather than trying (and failing) to stage the dirty content. PressPrimaryAction(). - /* EXPECTED: Lines( Equals(" M my_submodule_path (submodule)").IsSelected(), ) - ACTUAL: */ - Lines( - Equals("MM my_submodule_path (submodule)").IsSelected(), - ) }, })