Fix unstaging a submodule with dirty content#5666
Merged
Merged
Conversation
stefanhaller
changed the title
When a submodule has both a new commit (which the parent repo can stage) and dirty working-tree content (which it can't), staging it lands on a "MM" status. Pressing space again should unstage it, but instead it tries to stage the dirty content over and over, so you can never get back to an unstaged state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pressWithLock (acting on the selection) and toggleStagedAllWithLock (acting on the whole tree) each independently decided whether to stage or unstage, ran the optimistic update, and logged the action. That duplicated decision has already drifted: the tracked-files filter was added to press months before it was applied to stage-all, and fixes to one have repeatedly had to be chased into the other. Extract that shared decision into toggleStaged, leaving each caller to supply only the git commands it runs (per-path for the selection, bulk add -A / reset for the whole tree — the latter is required because the tree root node has an empty path, so a per-path stage wouldn't work). This is a pure refactor: the two callers' decisions were already equivalent, so behavior is unchanged. It exists so the next change to the staging logic only has to be made once. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Before the staging decision was unified, the stage (space) and stage-all (a) keybindings each made their own decision, so a fix to one wouldn't reach the other. Extend the test to drive the submodule through stage-all as well, guarding against that asymmetry coming back. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This map only feeds the optimistic rendering that makes staging feel instant; it doesn't affect the eventual status, which git reports after the refresh. The "MM" entry can never be reached for a regular file: a file at "MM" has stageable unstaged changes, so pressing space stages it rather than unstaging, and the unstage path is where this map is used. The only thing that reaches the unstage path at "MM" is a submodule whose commit is staged on top of dirty content, so this entry exists purely to update that submodule instantly instead of waiting for the next git status. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A submodule that only has dirty or untracked content (no new commit) can't be staged from the parent repo, but it still shows up as having unstaged changes. Pressing stage on it therefore briefly flashed as staged and then reverted, without explaining why nothing was staged. Detect this case (via `git submodule status`, where a '+' prefix marks a stageable commit change) in the shared stage/unstage decision: if the only thing that looks stageable is such a submodule, don't try to stage it. Instead unstage if there's anything staged to unstage, so the toggle stays symmetric; otherwise show an error explaining that there's nothing to stage. Because the decision is shared, this covers both the stage (space) and stage-all (a) keybindings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
stefanhaller
force-pushed
the
submodule-staging
branch
from
a6a4c2b to
785c8a7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Staging and unstaging submodules didn't work properly when the submodule had uncommitted changes of its own (modified or untracked files inside it). This PR fixes a few related problems:
Unstaging a submodule that has both a new commit and uncommitted changes now works. Previously, staging such a submodule left it half-staged, and from there the stage key would only ever try to re-stage it — there was no way to unstage it again. Now the stage key toggles it back to unstaged as expected.
Trying to stage a submodule that has nothing stageable now explains why. When a submodule's only changes are uncommitted content inside it (with no new commit), there's nothing the parent repository can stage. Instead of the keypress silently doing nothing (except briefly flashing the status to staged and then back to unstaged), lazygit now shows an error explaining that you need to commit inside the submodule first.
The stage key (space) and the stage-all key (
a) now behave consistently. All of the above applies equally whether you act on the submodule directly or use "stage all", and "stage all" no longer gets stuck or behaves differently from the stage key just because a submodule with uncommitted changes is present in the list.Fixes #3641.