From 76ed2047959b1cad5c8b9a8dbe9f509a1ffb1ce4 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sat, 20 May 2023 16:02:22 +1000 Subject: Add convenience builder for git commands Adds a convenience builder for git commands. Applies it in a couple of places that benefit from it. I've got a larger PR that does a more sweeping change but I want some reviews on that before I go ahead with it. --- pkg/commands/git_commands/branch_test.go | 52 ++++++++++-- pkg/commands/git_commands/commit_file_loader.go | 19 +++-- pkg/commands/git_commands/commit_loader.go | 82 +++++++++---------- pkg/commands/git_commands/deps_test.go | 6 ++ pkg/commands/git_commands/flow_test.go | 92 ++++++++++++++++++++++ pkg/commands/git_commands/git_command_builder.go | 65 +++++++++++++++ .../git_commands/git_command_builder_test.go | 56 +++++++++++++ 7 files changed, 314 insertions(+), 58 deletions(-) create mode 100644 pkg/commands/git_commands/flow_test.go create mode 100644 pkg/commands/git_commands/git_command_builder.go create mode 100644 pkg/commands/git_commands/git_command_builder_test.go diff --git a/pkg/commands/git_commands/branch_test.go b/pkg/commands/git_commands/branch_test.go index 2fdf7d9c2..75c288203 100644 --- a/pkg/commands/git_commands/branch_test.go +++ b/pkg/commands/git_commands/branch_test.go @@ -5,6 +5,7 @@ import ( "github.com/go-errors/errors" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/jesseduffield/lazygit/pkg/config" "github.com/stretchr/testify/assert" ) @@ -99,12 +100,53 @@ func TestBranchDeleteBranch(t *testing.T) { } func TestBranchMerge(t *testing.T) { - runner := oscommands.NewFakeRunner(t). - Expect(`git merge --no-edit "test"`, "", nil) - instance := buildBranchCommands(commonDeps{runner: runner}) + scenarios := []struct { + testName string + userConfig *config.UserConfig + opts MergeOpts + branchName string + expected string + }{ + { + testName: "basic", + userConfig: &config.UserConfig{}, + opts: MergeOpts{}, + branchName: "mybranch", + expected: `git merge --no-edit "mybranch"`, + }, + { + testName: "merging args", + userConfig: &config.UserConfig{ + Git: config.GitConfig{ + Merging: config.MergingConfig{ + Args: "--merging-args", // it's up to the user what they put here + }, + }, + }, + opts: MergeOpts{}, + branchName: "mybranch", + expected: `git merge --no-edit --merging-args "mybranch"`, + }, + { + testName: "fast forward only", + userConfig: &config.UserConfig{}, + opts: MergeOpts{FastForwardOnly: true}, + branchName: "mybranch", + expected: `git merge --no-edit --ff-only "mybranch"`, + }, + } - assert.NoError(t, instance.Merge("test", MergeOpts{})) - runner.CheckForMissingCalls() + for _, s := range scenarios { + s := s + t.Run(s.testName, func(t *testing.T) { + runner := oscommands.NewFakeRunner(t). + Expect(s.expected, "", nil) + instance := buildBranchCommands(commonDeps{runner: runner, userConfig: s.userConfig}) + + assert.NoError(t, instance.Merge(s.branchName, s.opts)) + runner.CheckForMissingCalls() + }) + } } func TestBranchCheckout(t *testing.T) { diff --git a/pkg/commands/git_commands/commit_file_loader.go b/pkg/commands/git_commands/commit_file_loader.go index 0b606ae86..53ca046ba 100644 --- a/pkg/commands/git_commands/commit_file_loader.go +++ b/pkg/commands/git_commands/commit_file_loader.go @@ -1,7 +1,6 @@ package git_commands import ( - "fmt" "strings" "github.com/jesseduffield/generics/slices" @@ -25,12 +24,18 @@ func NewCommitFileLoader(common *common.Common, cmd oscommands.ICmdObjBuilder) * // GetFilesInDiff get the specified commit files func (self *CommitFileLoader) GetFilesInDiff(from string, to string, reverse bool) ([]*models.CommitFile, error) { - reverseFlag := "" - if reverse { - reverseFlag = " -R " - } - - filenames, err := self.cmd.New(fmt.Sprintf("git diff --submodule --no-ext-diff --name-status -z --no-renames %s %s %s", reverseFlag, from, to)).DontLog().RunWithOutput() + cmdStr := NewGitCmd("diff"). + Arg("--submodule"). + Arg("--no-ext-diff"). + Arg("--name-status"). + Arg("-z"). + Arg("--no-renames"). + ArgIf(reverse, "-R"). + Arg(from). + Arg(to). + ToString() + + filenames, err := self.cmd.New(cmdStr).DontLog().RunWithOutput() if err != nil { return nil, err } diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index e68f56ebb..4fc6050ca 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -201,12 +201,14 @@ func (self *CommitLoader) getHydratedRebasingCommits(rebaseMode enums.RebaseMode // note that we're not filtering these as we do non-rebasing commits just because // I suspect that will cause some damage cmdObj := self.cmd.New( - fmt.Sprintf( - "git -c log.showSignature=false show %s --no-patch --oneline %s --abbrev=%d", - strings.Join(commitShas, " "), - prettyFormat, - 20, - ), + NewGitCmd("show"). + Config("log.showSignature=false"). + Arg("--no-patch"). + Arg("--oneline"). + Arg(strings.Join(commitShas, " ")). + Arg(prettyFormat). + Arg("--abbrev=20"). + ToString(), ).DontLog() fullCommits := map[string]*models.Commit{} @@ -375,8 +377,13 @@ func (self *CommitLoader) getMergeBase(refName string) string { // We pass all configured main branches to the merge-base call; git will // return the base commit for the closest one. - output, err := self.cmd.New(fmt.Sprintf("git merge-base %s %s", - self.cmd.Quote(refName), *self.quotedMainBranches)).DontLog().RunWithOutput() + + output, err := self.cmd.New( + NewGitCmd("merge-base"). + Arg(self.cmd.Quote(refName)). + Arg(*self.quotedMainBranches). + ToString(), + ).DontLog().RunWithOutput() if err != nil { // If there's an error, it must be because one of the main branches that // used to exist when we called getExistingMainBranches() was deleted @@ -391,7 +398,9 @@ func (self *CommitLoader) getExistingMainBranches() string { lo.FilterMap(self.UserConfig.Git.MainBranches, func(branchName string, _ int) (string, bool) { quotedRef := self.cmd.Quote("refs/heads/" + branchName) - if err := self.cmd.New(fmt.Sprintf("git rev-parse --verify --quiet %s", quotedRef)).DontLog().Run(); err != nil { + if err := self.cmd.New( + NewGitCmd("rev-parse").Arg("--verify").Arg("--quiet").Arg(quotedRef).ToString(), + ).DontLog().Run(); err != nil { return "", false } return quotedRef, true @@ -413,9 +422,10 @@ func ignoringWarnings(commandOutput string) string { func (self *CommitLoader) getFirstPushedCommit(refName string) (string, error) { output, err := self.cmd. New( - fmt.Sprintf("git merge-base %s %s@{u}", - self.cmd.Quote(refName), - self.cmd.Quote(strings.TrimPrefix(refName, "refs/heads/"))), + NewGitCmd("merge-base"). + Arg(self.cmd.Quote(refName)). + Arg(self.cmd.Quote(strings.TrimPrefix(refName, "refs/heads/")) + "@{u}"). + ToString(), ). DontLog(). RunWithOutput() @@ -428,42 +438,22 @@ func (self *CommitLoader) getFirstPushedCommit(refName string) (string, error) { // getLog gets the git log. func (self *CommitLoader) getLogCmd(opts GetCommitsOptions) oscommands.ICmdObj { - limitFlag := "" - if opts.Limit { - limitFlag = " -300" - } - - followFlag := "" - filterFlag := "" - if opts.FilterPath != "" { - followFlag = " --follow" - filterFlag = fmt.Sprintf(" %s", self.cmd.Quote(opts.FilterPath)) - } - config := self.UserConfig.Git.Log - orderFlag := "" - if config.Order != "default" { - orderFlag = " --" + config.Order - } - allFlag := "" - if opts.All { - allFlag = " --all" - } - - return self.cmd.New( - fmt.Sprintf( - "git log %s%s%s --oneline %s%s --abbrev=%d%s --no-show-signature --%s", - self.cmd.Quote(opts.RefName), - orderFlag, - allFlag, - prettyFormat, - limitFlag, - 40, - followFlag, - filterFlag, - ), - ).DontLog() + cmdStr := NewGitCmd("log"). + Arg(self.cmd.Quote(opts.RefName)). + ArgIf(config.Order != "default", "--"+config.Order). + ArgIf(opts.All, "--all"). + Arg("--oneline"). + Arg(prettyFormat). + Arg("--abbrev=40"). + ArgIf(opts.Limit, "-300"). + ArgIf(opts.FilterPath != "", "--follow"). + ArgIf(opts.FilterPath != "", self.cmd.Quote(opts.FilterPath)). + Arg("--no-show-signature"). + ToString() + + return self.cmd.New(cmdStr) } const prettyFormat = `--pretty=format:"%H%x00%at%x00%aN%x00%ae%x00%d%x00%p%x00%s"` diff --git a/pkg/commands/git_commands/deps_test.go b/pkg/commands/git_commands/deps_test.go index bcf36b168..6a20c72c2 100644 --- a/pkg/commands/git_commands/deps_test.go +++ b/pkg/commands/git_commands/deps_test.go @@ -150,3 +150,9 @@ func buildBranchCommands(deps commonDeps) *BranchCommands { return NewBranchCommands(gitCommon) } + +func buildFlowCommands(deps commonDeps) *FlowCommands { + gitCommon := buildGitCommon(deps) + + return NewFlowCommands(gitCommon) +} diff --git a/pkg/commands/git_commands/flow_test.go b/pkg/commands/git_commands/flow_test.go new file mode 100644 index 000000000..3ee2e91a5 --- /dev/null +++ b/pkg/commands/git_commands/flow_test.go @@ -0,0 +1,92 @@ +package git_commands + +import ( + "testing" + + "github.com/jesseduffield/lazygit/pkg/commands/git_config" + "github.com/stretchr/testify/assert" +) + +func TestStartCmdObj(t *testing.T) { + scenarios := []struct { + testName string + branchType string + name string + expected string + }{ + { + testName: "basic", + branchType: "feature", + name: "test", + expected: "git flow feature start test", + }, + } + + for _, s := range scenarios { + s := s + t.Run(s.testName, func(t *testing.T) { + instance := buildFlowCommands(commonDeps{}) + + assert.Equal(t, + instance.StartCmdObj(s.branchType, s.name).ToString(), + s.expected, + ) + }) + } +} + +func TestFinishCmdObj(t *testing.T) { + scenarios := []struct { + testName string + branchName string + expected string + expectedError string + gitConfigMockResponses map[string]string + }{ + { + testName: "not a git flow branch", + branchName: "mybranch", + expected: "", + expectedError: "This does not seem to be a git flow branch", + gitConfigMockResponses: nil, + }, + { + testName: "feature branch without config", + branchName: "feature/mybranch", + expected: "", + expectedError: "This does not seem to be a git flow branch", + gitConfigMockResponses: nil, + }, + { + testName: "feature branch with config", + branchName: "feature/mybranch", + expected: "git flow feature finish mybranch", + expectedError: "", + gitConfigMockResponses: map[string]string{ + "--local --get-regexp gitflow.prefix": "gitflow.prefix.feature feature/", + }, + }, + } + + for _, s := range scenarios { + s := s + t.Run(s.testName, func(t *testing.T) { + instance := buildFlowCommands(commonDeps{ + gitConfig: git_config.NewFakeGitConfig(s.gitConfigMockResponses), + }) + + cmd, err := instance.FinishCmdObj(s.branchName) + + if s.expectedError != "" { + if err == nil { + t.Errorf("Expected error, got nil") + } else { + assert.Equal(t, err.Error(), s.expectedError) + } + } else { + assert.NoError(t, err) + assert.Equal(t, cmd.ToString(), s.expected) + } + }) + } +} diff --git a/pkg/commands/git_commands/git_command_builder.go b/pkg/commands/git_commands/git_command_builder.go new file mode 100644 index 000000000..7708c8e9f --- /dev/null +++ b/pkg/commands/git_commands/git_command_builder.go @@ -0,0 +1,65 @@ +package git_commands + +import "fmt" + +// convenience struct for building git commands. Especially useful when +// including conditional args +type GitCommandBuilder struct { + command string +} + +func NewGitCmd(command string) *GitCommandBuilder { + return &GitCommandBuilder{command: command} +} + +func (self *GitCommandBuilder) Arg(flag string) *GitCommandBuilder { + if flag == "" { + return self + } + + self.command += " " + flag + + return self +} + +func (self *GitCommandBuilder) ArgIf(include bool, flag string) *GitCommandBuilder { + if include { + return self.Arg(flag) + } + + return self +} + +func (self *GitCommandBuilder) ArgIfElse(isTrue bool, onTrue string, onFalse string) *GitCommandBuilder { + if isTrue { + return self.Arg(onTrue) + } else { + return self.Arg(onFalse) + } +} + +func (self *GitCommandBuilder) Args(args []string) *GitCommandBuilder { + for _, arg := range args { + self.Arg(arg) + } + + return self +} + +func (self *GitCommandBuilder) Config(value string) *GitCommandBuilder { + // config settings come before the command + self.command = fmt.Sprintf("-c %s %s", value, self.command) + + return self +} + +func (self *GitCommandBuilder) RepoPath(value string) *GitCommandBuilder { + // repo path comes before the command + self.command = fmt.Sprintf("-C %s %s", value, self.command) + + return self +} + +func (self *GitCommandBuilder) ToString() string { + return "git " + self.command +} diff --git a/pkg/commands/git_commands/git_command_builder_test.go b/pkg/commands/git_commands/git_command_builder_test.go new file mode 100644 index 000000000..e3bc55ccf --- /dev/null +++ b/pkg/commands/git_commands/git_command_builder_test.go @@ -0,0 +1,56 @@ +package git_commands + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGitCommandBuilder(t *testing.T) { + scenarios := []struct { + input string + expected string + }{ + { + input: NewGitCmd("push"). + Arg("--force-with-lease"). + Arg("--set-upstream"). + Arg("origin"). + Arg("master"). + ToString(), + expected: "git push --force-with-lease --set-upstream origin master", + }, + { + input: NewGitCmd("push").ArgIf(true, "--test").ToString(), + expected: "git push --test", + }, + { + input: NewGitCmd("push").ArgIf(false, "--test").ToString(), + expected: "git push", + }, + { + input: NewGitCmd("push").ArgIfElse(true, "-b", "-a").ToString(), + expected: "git push -b", + }, + { + input: NewGitCmd("push").ArgIfElse(false, "-a", "-b").ToString(), + expected: "git push -b", + }, + { + input: NewGitCmd("push").Args([]string{"-a", "-b"}).ToString(), + expected: "git push -a -b", + }, + { + input: NewGitCmd("push").Config("user.name=foo").Config("user.email=bar").ToString(), + expected: "git -c user.email=bar -c user.name=foo push", + }, + { + input: NewGitCmd("push").RepoPath("a/b/c").ToString(), + expected: "git -C a/b/c push", + }, + } + + for _, s := range scenarios { + assert.Equal(t, s.input, s.expected) + } +} -- cgit v1.2.3