Merge pull request #2585 from stefanhaller/only-use-empty-arg-when-available

Author: jesseduffield

pkg/commands/git_commands/rebase.go

@@ -181,12 +181,18 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract
 debug = "TRUE"
 }
 
+emptyArg := " --empty=keep"
+if self.version.IsOlderThan(2, 26, 0) {
+emptyArg = ""
+}
+
 rebaseMergesArg := " --rebase-merges"
 if self.version.IsOlderThan(2, 22, 0) {
 rebaseMergesArg = ""
 }
-cmdStr := fmt.Sprintf("git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash%s %s",
-rebaseMergesArg, opts.baseShaOrRoot)
+
+cmdStr := fmt.Sprintf("git rebase --interactive --autostash --keep-empty%s --no-autosquash%s %s",
+emptyArg, rebaseMergesArg, opts.baseShaOrRoot)
 self.Log.WithField("command", cmdStr).Debug("RunCommand")
 
 cmdObj := self.cmd.New(cmdStr)

pkg/commands/git_commands/rebase_test.go

@@ -16,37 +16,60 @@ import (
 
 func TestRebaseRebaseBranch(t *testing.T) {
 type scenario struct {
-testName string
-arg string
-runner *oscommands.FakeCmdObjRunner
-test func(error)
+testName string
+arg string
+gitVersion *GitVersion
+runner *oscommands.FakeCmdObjRunner
+test func(error)
 }
 
 scenarios := []scenario{
 {
-testName: "successful rebase",
-arg: "master",
+testName: "successful rebase",
+arg: "master",
+gitVersion: &GitVersion{2, 26, 0, ""},
 runner: oscommands.NewFakeRunner(t).
-Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash master`, "", nil),
+Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash --rebase-merges master`, "", nil),
 test: func(err error) {
 assert.NoError(t, err)
 },
 },
 {
-testName: "unsuccessful rebase",
-arg: "master",
+testName: "unsuccessful rebase",
+arg: "master",
+gitVersion: &GitVersion{2, 26, 0, ""},
 runner: oscommands.NewFakeRunner(t).
-Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash master`, "", errors.New("error")),
+Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash --rebase-merges master`, "", errors.New("error")),
 test: func(err error) {
 assert.Error(t, err)
 },
 },
+{
+testName: "successful rebase (< 2.26.0)",
+arg: "master",
+gitVersion: &GitVersion{2, 25, 5, ""},
+runner: oscommands.NewFakeRunner(t).
+Expect(`git rebase --interactive --autostash --keep-empty --no-autosquash --rebase-merges master`, "", nil),
+test: func(err error) {
+assert.NoError(t, err)
+},
+},
+{
+testName: "successful rebase (< 2.22.0)",
+arg: "master",
+gitVersion: &GitVersion{2, 21, 9, ""},
+runner: oscommands.NewFakeRunner(t).
+Expect(`git rebase --interactive --autostash --keep-empty --no-autosquash master`, "", nil),
+test: func(err error) {
+assert.NoError(t, err)
+},
+},
 }
 
 for _, s := range scenarios {
 s := s
 t.Run(s.testName, func(t *testing.T) {
-instance := buildRebaseCommands(commonDeps{runner: s.runner})
+instance := buildRebaseCommands(commonDeps{runner: s.runner, gitVersion: s.gitVersion})
 s.test(instance.RebaseBranch(s.arg))
 })
 }
@@ -126,7 +149,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
 commitIndex: 0,
 fileName: "test999.txt",
 runner: oscommands.NewFakeRunner(t).
-Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash abcdef`, "", nil).
+Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash --rebase-merges abcdef`, "", nil).
 Expect(`git cat-file -e HEAD^:"test999.txt"`, "", nil).
 Expect(`git checkout HEAD^ -- "test999.txt"`, "", nil).
 Expect(`git commit --amend --no-edit --allow-empty`, "", nil).
@@ -143,8 +166,9 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
 s := s
 t.Run(s.testName, func(t *testing.T) {
 instance := buildRebaseCommands(commonDeps{
-runner: s.runner,
-gitConfig: git_config.NewFakeGitConfig(s.gitConfigMockResponses),
+runner: s.runner,
+gitVersion: &GitVersion{2, 26, 0, ""},
+gitConfig: git_config.NewFakeGitConfig(s.gitConfigMockResponses),
 })
 
 s.test(instance.DiscardOldFileChanges(s.commits, s.commitIndex, s.fileName))

pkg/integration/components/test.go

@@ -60,7 +60,7 @@ type GitVersionRestriction struct {
 }
 
 // Verifies the version is at least the given version (inclusive)
-func From(version string) GitVersionRestriction {
+func AtLeast(version string) GitVersionRestriction {
 return GitVersionRestriction{from: version}
 }
 

pkg/integration/components/test_test.go

@@ -96,18 +96,18 @@ func TestGitVersionRestriction(t *testing.T) {
 expectedShouldRun bool
 }{
 {
-testName: "From, current is newer",
-gitVersion: From("2.24.9"),
+testName: "AtLeast, current is newer",
+gitVersion: AtLeast("2.24.9"),
 expectedShouldRun: true,
 },
 {
-testName: "From, current is same",
-gitVersion: From("2.25.0"),
+testName: "AtLeast, current is same",
+gitVersion: AtLeast("2.25.0"),
 expectedShouldRun: true,
 },
 {
-testName: "From, current is older",
-gitVersion: From("2.26.0"),
+testName: "AtLeast, current is older",
+gitVersion: AtLeast("2.26.0"),
 expectedShouldRun: false,
 },
 {

pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref.go

@@ -9,7 +9,7 @@ var DropTodoCommitWithUpdateRef = NewIntegrationTest(NewIntegrationTestArgs{
 Description: "Drops a commit during interactive rebase when there is an update-ref in the git-rebase-todo file",
 ExtraCmdArgs: "",
 Skip: false,
-GitVersion: From("2.38.0"),
+GitVersion: AtLeast("2.38.0"),
 SetupConfig: func(config *config.AppConfig) {},
 SetupRepo: func(shell *Shell) {
 shell.

pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref_show_branch_heads.go

@@ -9,7 +9,7 @@ var DropTodoCommitWithUpdateRefShowBranchHeads = NewIntegrationTest(NewIntegrati
 Description: "Drops a commit during interactive rebase when there is an update-ref in the git-rebase-todo file (with experimentalShowBranchHeads on)",
 ExtraCmdArgs: "",
 Skip: false,
-GitVersion: From("2.38.0"),
+GitVersion: AtLeast("2.38.0"),
 SetupConfig: func(config *config.AppConfig) {
 config.UserConfig.Gui.ExperimentalShowBranchHeads = true
 },

pkg/integration/tests/patch_building/move_to_earlier_commit.go

@@ -9,6 +9,7 @@ var MoveToEarlierCommit = NewIntegrationTest(NewIntegrationTestArgs{
 Description: "Move a patch from a commit to an earlier commit",
 ExtraCmdArgs: "",
 Skip: false,
+GitVersion: AtLeast("2.26.0"),
 SetupConfig: func(config *config.AppConfig) {},
 SetupRepo: func(shell *Shell) {
 shell.CreateDir("dir")

pkg/integration/tests/patch_building/move_to_earlier_commit_no_keep_empty.go

@@ -0,0 +1,77 @@
+package patch_building
+
+import (
+"github.com/jesseduffield/lazygit/pkg/config"
+. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var MoveToEarlierCommitNoKeepEmpty = NewIntegrationTest(NewIntegrationTestArgs{
+Description: "Move a patch from a commit to an earlier commit, for older git versions that don't keep the empty commit",
+ExtraCmdArgs: "",
+Skip: false,
+GitVersion: Before("2.26.0"),
+SetupConfig: func(config *config.AppConfig) {},
+SetupRepo: func(shell *Shell) {
+shell.CreateDir("dir")
+shell.CreateFileAndAdd("dir/file1", "file1 content")
+shell.CreateFileAndAdd("dir/file2", "file2 content")
+shell.Commit("first commit")
+
+shell.CreateFileAndAdd("unrelated-file", "")
+shell.Commit("destination commit")
+
+shell.UpdateFileAndAdd("dir/file1", "file1 content with old changes")
+shell.DeleteFileAndAdd("dir/file2")
+shell.CreateFileAndAdd("dir/file3", "file3 content")
+shell.Commit("commit to move from")
+},
+Run: func(t *TestDriver, keys config.KeybindingConfig) {
+t.Views().Commits().
+Focus().
+Lines(
+Contains("commit to move from").IsSelected(),
+Contains("destination commit"),
+Contains("first commit"),
+).
+PressEnter()
+
+t.Views().CommitFiles().
+IsFocused().
+Lines(
+Contains("dir").IsSelected(),
+Contains(" M file1"),
+Contains(" D file2"),
+Contains(" A file3"),
+).
+PressPrimaryAction().
+PressEscape()
+
+t.Views().Information().Content(Contains("building patch"))
+
+t.Views().Commits().
+IsFocused().
+SelectNextItem()
+
+t.Common().SelectPatchOption(Contains("move patch to selected commit"))
+
+t.Views().Commits().
+IsFocused().
+Lines(
+Contains("destination commit"),
+Contains("first commit").IsSelected(),
+).
+SelectPreviousItem().
+PressEnter()
+
+t.Views().CommitFiles().
+IsFocused().
+Lines(
+Contains("dir").IsSelected(),
+Contains(" M file1"),
+Contains(" D file2"),
+Contains(" A file3"),
+Contains("A unrelated-file"),
+).
+PressEscape()
+},
+})

pkg/integration/tests/test_list.go

@@ -117,6 +117,7 @@ var tests = []*components.IntegrationTest{
 patch_building.ApplyInReverseWithConflict,
 patch_building.CopyPatchToClipboard,
 patch_building.MoveToEarlierCommit,
+patch_building.MoveToEarlierCommitNoKeepEmpty,
 patch_building.MoveToIndex,
 patch_building.MoveToIndexPartOfAdjacentAddedLines,
 patch_building.MoveToIndexPartial,