mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-09-10 07:36:27 -04:00
Explain when a submodule has nothing stageable
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>
This commit is contained in:
parent
8b5cfb0425
commit
785c8a712c
|
|
@ -9,6 +9,7 @@ import (
|
|||
|
||||
"github.com/jesseduffield/lazygit/pkg/commands/models"
|
||||
"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
|
||||
"github.com/samber/lo"
|
||||
)
|
||||
|
||||
// .gitmodules looks like this:
|
||||
|
|
@ -86,6 +87,30 @@ func (self *SubmoduleCommands) GetConfigs(parentModule *models.SubmoduleConfig)
|
|||
return configs, nil
|
||||
}
|
||||
|
||||
// AnyHaveStageableChanges reports whether any of the given submodule paths has
|
||||
// a checked-out commit that differs from the one recorded in the
|
||||
// superproject's index, i.e. a change that `git add <path>` would actually
|
||||
// stage. A submodule that only has dirty or untracked content (with no new
|
||||
// commit) can't be staged from the superproject, so it won't be reported here.
|
||||
func (self *SubmoduleCommands) AnyHaveStageableChanges(paths []string) (bool, error) {
|
||||
if len(paths) == 0 {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
cmdArgs := NewGitCmd("submodule").Arg("status", "--").Arg(paths...).ToArgv()
|
||||
output, err := self.cmd.New(cmdArgs).DontLog().RunWithOutput()
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
// Each line looks like "<prefix><sha> <path> (<describe>)". A '+' prefix
|
||||
// means the checked-out commit differs from the index, i.e. there's a
|
||||
// commit change to stage.
|
||||
return lo.SomeBy(strings.Split(output, "\n"), func(line string) bool {
|
||||
return strings.HasPrefix(line, "+")
|
||||
}), nil
|
||||
}
|
||||
|
||||
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
|
||||
|
|
|
|||
|
|
@ -476,7 +476,22 @@ func (self *FilesController) toggleStaged(
|
|||
|
||||
unstagedNodes := filterNodesHaveUnstagedChanges(nodes, self.c.Model().Submodules)
|
||||
|
||||
if len(unstagedNodes) > 0 {
|
||||
// Staging a submodule that only has dirty or untracked content (no new
|
||||
// commit) is a no-op: the parent repo can't stage that content. When that's
|
||||
// the only thing that looks stageable, don't stage; fall through to
|
||||
// unstaging instead. That keeps the toggle symmetric (e.g. a fully-staged
|
||||
// tree that also contains a dirty submodule still unstages on the next
|
||||
// press) rather than getting stuck trying to stage the unstageable content.
|
||||
shouldStage := len(unstagedNodes) > 0
|
||||
if shouldStage {
|
||||
noOp, err := self.stagingWouldBeNoOp(unstagedNodes)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
shouldStage = !noOp
|
||||
}
|
||||
|
||||
if shouldStage {
|
||||
self.c.LogAction(stageAction)
|
||||
|
||||
if err := self.optimisticChange(unstagedNodes, self.optimisticStage); err != nil {
|
||||
|
|
@ -486,6 +501,12 @@ func (self *FilesController) toggleStaged(
|
|||
return stage(unstagedNodes)
|
||||
}
|
||||
|
||||
// If there's nothing staged to unstage either, then the only thing we acted
|
||||
// on was an unstageable submodule and nothing happened, so say why.
|
||||
if !someNodesHaveStagedChanges(nodes) {
|
||||
return errors.New(self.c.Tr.NothingToStageForSubmodule)
|
||||
}
|
||||
|
||||
self.c.LogAction(unstageAction)
|
||||
|
||||
if err := self.optimisticChange(nodes, self.optimisticUnstage); err != nil {
|
||||
|
|
@ -1450,6 +1471,40 @@ func fileHasStageableUnstagedChanges(file *models.File, submodules []*models.Sub
|
|||
return true
|
||||
}
|
||||
|
||||
// stagingWouldBeNoOp reports whether staging the given nodes would have no
|
||||
// visible effect, which happens when the only things being staged are
|
||||
// submodules that have dirty or untracked content but no new commit: the
|
||||
// parent repo can't stage that content. If a regular file (or a submodule with
|
||||
// a stageable new commit) is among them, staging does something, so this
|
||||
// returns false.
|
||||
func (self *FilesController) stagingWouldBeNoOp(nodes []*filetree.FileNode) (bool, error) {
|
||||
submodules := self.c.Model().Submodules
|
||||
|
||||
var submodulePaths []string
|
||||
hasOtherStageableChanges := false
|
||||
for _, node := range nodes {
|
||||
_ = node.ForEachFile(func(file *models.File) error {
|
||||
if file.IsSubmodule(submodules) {
|
||||
submodulePaths = append(submodulePaths, file.Path)
|
||||
} else if file.HasUnstagedChanges {
|
||||
hasOtherStageableChanges = true
|
||||
}
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
if hasOtherStageableChanges || len(submodulePaths) == 0 {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
anyStageable, err := self.c.Git().Submodule.AnyHaveStageableChanges(submodulePaths)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
return !anyStageable, nil
|
||||
}
|
||||
|
||||
func findSubmoduleNode(nodes []*filetree.FileNode, submodules []*models.SubmoduleConfig) *models.File {
|
||||
for _, node := range nodes {
|
||||
submoduleNode := node.FindFirstFileBy(func(f *models.File) bool {
|
||||
|
|
|
|||
|
|
@ -917,6 +917,7 @@ type TranslationSet struct {
|
|||
SelectedItemIsNotABranch string
|
||||
SelectedItemDoesNotHaveFiles string
|
||||
MultiSelectNotSupportedForSubmodules string
|
||||
NothingToStageForSubmodule string
|
||||
CommandDoesNotSupportOpeningInEditor string
|
||||
CustomCommands string
|
||||
NoApplicableCommandsInThisContext string
|
||||
|
|
@ -2038,6 +2039,7 @@ func EnglishTranslationSet() *TranslationSet {
|
|||
SelectedItemIsNotABranch: "Selected item is not a branch",
|
||||
SelectedItemDoesNotHaveFiles: "Selected item does not have files to view",
|
||||
MultiSelectNotSupportedForSubmodules: "Multiselection not supported for submodules",
|
||||
NothingToStageForSubmodule: "Nothing to stage: the parent repo can only stage a new submodule commit, not the uncommitted changes inside a submodule. Commit inside the submodule first.",
|
||||
CommandDoesNotSupportOpeningInEditor: "This command doesn't support switching to the editor",
|
||||
CustomCommands: "Custom commands",
|
||||
NoApplicableCommandsInThisContext: "(No applicable commands in this context)",
|
||||
|
|
|
|||
|
|
@ -0,0 +1,46 @@
|
|||
package submodule
|
||||
|
||||
import (
|
||||
"github.com/jesseduffield/lazygit/pkg/config"
|
||||
. "github.com/jesseduffield/lazygit/pkg/integration/components"
|
||||
)
|
||||
|
||||
var StageAllWithDirtySubmodule = NewIntegrationTest(NewIntegrationTestArgs{
|
||||
Description: "A submodule with only dirty content (which can't be staged) must not break the stage-all toggle: pressing it repeatedly should keep toggling the other files between staged and unstaged.",
|
||||
ExtraCmdArgs: []string{},
|
||||
Skip: false,
|
||||
SetupConfig: func(config *config.AppConfig) {
|
||||
config.GetUserConfig().Gui.ShowFileTree = false
|
||||
},
|
||||
SetupRepo: func(shell *Shell) {
|
||||
shell.EmptyCommit("first commit")
|
||||
shell.CloneIntoSubmodule("my_submodule_name", "my_submodule_path")
|
||||
shell.GitAddAll()
|
||||
shell.Commit("add submodule")
|
||||
|
||||
// A submodule with dirty content but no new commit (can't be staged),
|
||||
// alongside a regular file that can.
|
||||
shell.CreateFile("my_submodule_path/dirty_file", "dirty content")
|
||||
shell.CreateFile("regular_file", "content")
|
||||
},
|
||||
Run: func(t *TestDriver, keys config.KeybindingConfig) {
|
||||
t.Views().Files().Focus().
|
||||
Lines(
|
||||
Equals(" M my_submodule_path (submodule)"),
|
||||
Equals("?? regular_file"),
|
||||
).
|
||||
// Stage all: the regular file gets staged; the submodule can't be.
|
||||
Press(keys.Files.ToggleStagedAll).
|
||||
Lines(
|
||||
Equals(" M my_submodule_path (submodule)"),
|
||||
Equals("A regular_file"),
|
||||
).
|
||||
// Stage all again: nothing is stageable, but the regular file is
|
||||
// staged, so this unstages it rather than erroring on the submodule.
|
||||
Press(keys.Files.ToggleStagedAll).
|
||||
Lines(
|
||||
Equals(" M my_submodule_path (submodule)"),
|
||||
Equals("?? regular_file"),
|
||||
)
|
||||
},
|
||||
})
|
||||
53
pkg/integration/tests/submodule/stage_dirty_only.go
Normal file
53
pkg/integration/tests/submodule/stage_dirty_only.go
Normal file
|
|
@ -0,0 +1,53 @@
|
|||
package submodule
|
||||
|
||||
import (
|
||||
"github.com/jesseduffield/lazygit/pkg/config"
|
||||
. "github.com/jesseduffield/lazygit/pkg/integration/components"
|
||||
)
|
||||
|
||||
var StageDirtyOnly = NewIntegrationTest(NewIntegrationTestArgs{
|
||||
Description: "Pressing space on a submodule that only has dirty content (no new commit) can't stage anything, so we explain that with an error instead of silently doing nothing.",
|
||||
ExtraCmdArgs: []string{},
|
||||
Skip: false,
|
||||
SetupConfig: func(config *config.AppConfig) {
|
||||
config.GetUserConfig().Gui.ShowFileTree = false
|
||||
},
|
||||
SetupRepo: func(shell *Shell) {
|
||||
shell.EmptyCommit("first commit")
|
||||
shell.CloneIntoSubmodule("my_submodule_name", "my_submodule_path")
|
||||
shell.GitAddAll()
|
||||
shell.Commit("add submodule")
|
||||
|
||||
// Dirty working-tree content, but no new commit: there's nothing the
|
||||
// parent repo can stage.
|
||||
shell.CreateFile("my_submodule_path/dirty_file", "dirty content")
|
||||
},
|
||||
Run: func(t *TestDriver, keys config.KeybindingConfig) {
|
||||
t.Views().Files().Focus().
|
||||
Lines(
|
||||
Equals(" M my_submodule_path (submodule)").IsSelected(),
|
||||
).
|
||||
PressPrimaryAction().
|
||||
Tap(func() {
|
||||
t.ExpectPopup().Alert().
|
||||
Title(Equals("Error")).
|
||||
Content(Contains("Nothing to stage")).
|
||||
Confirm()
|
||||
}).
|
||||
// The status is unchanged: nothing got staged.
|
||||
Lines(
|
||||
Equals(" M my_submodule_path (submodule)").IsSelected(),
|
||||
).
|
||||
// Pressing "stage all" must behave the same way.
|
||||
Press(keys.Files.ToggleStagedAll).
|
||||
Tap(func() {
|
||||
t.ExpectPopup().Alert().
|
||||
Title(Equals("Error")).
|
||||
Content(Contains("Nothing to stage")).
|
||||
Confirm()
|
||||
}).
|
||||
Lines(
|
||||
Equals(" M my_submodule_path (submodule)").IsSelected(),
|
||||
)
|
||||
},
|
||||
})
|
||||
|
|
@ -427,6 +427,8 @@ var tests = []*components.IntegrationTest{
|
|||
submodule.Reset,
|
||||
submodule.ResetFolder,
|
||||
submodule.Stage,
|
||||
submodule.StageAllWithDirtySubmodule,
|
||||
submodule.StageDirtyOnly,
|
||||
sync.FetchAndAutoForwardBranchesAllBranches,
|
||||
sync.FetchAndAutoForwardBranchesAllBranchesCheckedOutInOtherWorktree,
|
||||
sync.FetchAndAutoForwardBranchesNone,
|
||||
|
|
|
|||
Loading…
Reference in a new issue