Construct arg vector manually rather than parse string

By constructing an arg vector manually, we no longer need to quote arguments

Mandate that args must be passed when building a command

Now you need to provide an args array when building a command.
There are a handful of places where we need to deal with a string,
such as with user-defined custom commands, and for those we now require
that at the callsite they use str.ToArgv to do that. I don't want
to provide a method out of the box for it because I want to discourage its
use.

For some reason we were invoking a command through a shell when amending a
commit, and I don't believe we needed to do that as there was nothing user-
supplied about the command. So I've switched to using a regular command out-
side the shell there

49 files changed, 716 insertions, 628 deletions

diff --git a/pkg/commands/git.go b/pkg/commands/git.go
index 95e46086f..2ae8694e7 100644
--- a/pkg/commands/git.go
+++ b/pkg/commands/git.go
@@ -24,6 +24,7 @@ type GitCommand struct {
 	Commit      *git_commands.CommitCommands
 	Config      *git_commands.ConfigCommands
 	Custom      *git_commands.CustomCommands
+	Diff        *git_commands.DiffCommands
 	File        *git_commands.FileCommands
 	Flow        *git_commands.FlowCommands
 	Patch       *git_commands.PatchCommands
@@ -112,6 +113,7 @@ func NewGitCommandAux(
 	tagCommands := git_commands.NewTagCommands(gitCommon)
 	commitCommands := git_commands.NewCommitCommands(gitCommon)
 	customCommands := git_commands.NewCustomCommands(gitCommon)
+	diffCommands := git_commands.NewDiffCommands(gitCommon)
 	fileCommands := git_commands.NewFileCommands(gitCommon)
 	submoduleCommands := git_commands.NewSubmoduleCommands(gitCommon)
 	workingTreeCommands := git_commands.NewWorkingTreeCommands(gitCommon, submoduleCommands, fileLoader)
@@ -139,6 +141,7 @@ func NewGitCommandAux(
 		Commit:      commitCommands,
 		Config:      configCommands,
 		Custom:      customCommands,
+		Diff:        diffCommands,
 		File:        fileCommands,
 		Flow:        flowCommands,
 		Patch:       patchCommands,
@@ -274,5 +277,5 @@ func findDotGitDir(stat func(string) (os.FileInfo, error), readFile func(filenam
 }
 
 func VerifyInGitRepo(osCommand *oscommands.OSCommand) error {
-	return osCommand.Cmd.New("git rev-parse --git-dir").DontLog().Run()
+	return osCommand.Cmd.New(git_commands.NewGitCmd("rev-parse").Arg("--git-dir").ToArgv()).DontLog().Run()
 }
diff --git a/pkg/commands/git_cmd_obj_builder.go b/pkg/commands/git_cmd_obj_builder.go
index 487dd2303..21f300bb3 100644
--- a/pkg/commands/git_cmd_obj_builder.go
+++ b/pkg/commands/git_cmd_obj_builder.go
@@ -30,12 +30,8 @@ func NewGitCmdObjBuilder(log *logrus.Entry, innerBuilder *oscommands.CmdObjBuild
 
 var defaultEnvVar = "GIT_OPTIONAL_LOCKS=0"
 
-func (self *gitCmdObjBuilder) New(cmdStr string) oscommands.ICmdObj {
-	return self.innerBuilder.New(cmdStr).AddEnvVars(defaultEnvVar)
-}
-
-func (self *gitCmdObjBuilder) NewFromArgs(args []string) oscommands.ICmdObj {
-	return self.innerBuilder.NewFromArgs(args).AddEnvVars(defaultEnvVar)
+func (self *gitCmdObjBuilder) New(args []string) oscommands.ICmdObj {
+	return self.innerBuilder.New(args).AddEnvVars(defaultEnvVar)
 }
 
 func (self *gitCmdObjBuilder) NewShell(cmdStr string) oscommands.ICmdObj {
diff --git a/pkg/commands/git_commands/bisect.go b/pkg/commands/git_commands/bisect.go
index 898151d9c..101f2037b 100644
--- a/pkg/commands/git_commands/bisect.go
+++ b/pkg/commands/git_commands/bisect.go
@@ -97,15 +97,15 @@ func (self *BisectCommands) GetInfo() *BisectInfo {
 }
 
 func (self *BisectCommands) Reset() error {
-	cmdStr := NewGitCmd("bisect").Arg("reset").ToString()
+	cmdArgs := NewGitCmd("bisect").Arg("reset").ToArgv()
 
-	return self.cmd.New(cmdStr).StreamOutput().Run()
+	return self.cmd.New(cmdArgs).StreamOutput().Run()
 }
 
 func (self *BisectCommands) Mark(ref string, term string) error {
-	cmdStr := NewGitCmd("bisect").Arg(term, ref).ToString()
+	cmdArgs := NewGitCmd("bisect").Arg(term, ref).ToArgv()
 
-	return self.cmd.New(cmdStr).
+	return self.cmd.New(cmdArgs).
 		IgnoreEmptyError().
 		StreamOutput().
 		Run()
@@ -116,9 +116,9 @@ func (self *BisectCommands) Skip(ref string) error {
 }
 
 func (self *BisectCommands) Start() error {
-	cmdStr := NewGitCmd("bisect").Arg("start").ToString()
+	cmdArgs := NewGitCmd("bisect").Arg("start").ToArgv()
 
-	return self.cmd.New(cmdStr).StreamOutput().Run()
+	return self.cmd.New(cmdArgs).StreamOutput().Run()
 }
 
 // tells us whether we've found our problem commit(s). We return a string slice of
@@ -140,8 +140,8 @@ func (self *BisectCommands) IsDone() (bool, []string, error) {
 	done := false
 	candidates := []string{}
 
-	cmdStr := NewGitCmd("rev-list").Arg(newSha).ToString()
-	err := self.cmd.New(cmdStr).RunAndProcessLines(func(line string) (bool, error) {
+	cmdArgs := NewGitCmd("rev-list").Arg(newSha).ToArgv()
+	err := self.cmd.New(cmdArgs).RunAndProcessLines(func(line string) (bool, error) {
 		sha := strings.TrimSpace(line)
 
 		if status, ok := info.statusMap[sha]; ok {
@@ -171,11 +171,11 @@ func (self *BisectCommands) IsDone() (bool, []string, error) {
 // bisecting is actually a descendant of our current bisect commit. If it's not, we need to
 // render the commits from the bad commit.
 func (self *BisectCommands) ReachableFromStart(bisectInfo *BisectInfo) bool {
-	cmdStr := NewGitCmd("merge-base").
+	cmdArgs := NewGitCmd("merge-base").
 		Arg("--is-ancestor", bisectInfo.GetNewSha(), bisectInfo.GetStartSha()).
-		ToString()
+		ToArgv()
 
-	err := self.cmd.New(cmdStr).DontLog().Run()
+	err := self.cmd.New(cmdArgs).DontLog().Run()
 
 	return err == nil
 }
diff --git a/pkg/commands/git_commands/branch.go b/pkg/commands/git_commands/branch.go
index 0952e59e1..3c4d97c82 100644
--- a/pkg/commands/git_commands/branch.go
+++ b/pkg/commands/git_commands/branch.go
@@ -6,6 +6,7 @@ import (
 
 	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
 	"github.com/jesseduffield/lazygit/pkg/utils"
+	"github.com/mgutz/str"
 )
 
 type BranchCommands struct {
@@ -20,11 +21,11 @@ func NewBranchCommands(gitCommon *GitCommon) *BranchCommands {
 
 // New creates a new branch
 func (self *BranchCommands) New(name string, base string) error {
-	cmdStr := NewGitCmd("checkout").
-		Arg("-b", self.cmd.Quote(name), self.cmd.Quote(base)).
-		ToString()
+	cmdArgs := NewGitCmd("checkout").
+		Arg("-b", name, base).
+		ToArgv()
 
-	return self.cmd.New(cmdStr).Run()
+	return self.cmd.New(cmdArgs).Run()
 }
 
 // CurrentBranchInfo get the current branch information.
@@ -32,7 +33,7 @@ func (self *BranchCommands) CurrentBranchInfo() (BranchInfo, error) {
 	branchName, err := self.cmd.New(
 		NewGitCmd("symbolic-ref").
 			Arg("--short", "HEAD").
-			ToString(),
+			ToArgv(),
 	).DontLog().RunWithOutput()
 	if err == nil && branchName != "HEAD\n" {
 		trimmedBranchName := strings.TrimSpace(branchName)
@@ -44,8 +45,8 @@ func (self *BranchCommands) CurrentBranchInfo() (BranchInfo, error) {
 	}
 	output, err := self.cmd.New(
 		NewGitCmd("branch").
-			Arg("--points-at=HEAD", "--format=\"%(HEAD)%00%(objectname)%00%(refname)\"").
-			ToString(),
+			Arg("--points-at=HEAD", "--format=%(HEAD)%00%(objectname)%00%(refname)").
+			ToArgv(),
 	).DontLog().RunWithOutput()
 	if err != nil {
 		return BranchInfo{}, err
@@ -71,12 +72,12 @@ func (self *BranchCommands) CurrentBranchInfo() (BranchInfo, error) {
 
 // Delete delete branch
 func (self *BranchCommands) Delete(branch string, force bool) error {
-	cmdStr := NewGitCmd("branch").
+	cmdArgs := NewGitCmd("branch").
 		ArgIfElse(force, "-D", "-d").
-		Arg(self.cmd.Quote(branch)).
-		ToString()
+		Arg(branch).
+		ToArgv()
 
-	return self.cmd.New(cmdStr).Run()
+	return self.cmd.New(cmdArgs).Run()
 }
 
 // Checkout checks out a branch (or commit), with --force if you set the force arg to true
@@ -86,12 +87,12 @@ type CheckoutOptions struct {
 }
 
 func (self *BranchCommands) Checkout(branch string, options CheckoutOptions) error {
-	cmdStr := NewGitCmd("checkout").
+	cmdArgs := NewGitCmd("checkout").
 		ArgIf(options.Force, "--force").
-		Arg(self.cmd.Quote(branch)).
-		ToString()
+		Arg(branch).
+		ToArgv()
 
-	return self.cmd.New(cmdStr).
+	return self.cmd.New(cmdArgs).
 		// prevents git from prompting us for input which would freeze the program
 		// TODO: see if this is actually needed here
 		AddEnvVars("GIT_TERMINAL_PROMPT=0").
@@ -111,31 +112,34 @@ func (self *BranchCommands) GetGraphCmdObj(branchName string) oscommands.ICmdObj
 	templateValues := map[string]string{
 		"branchName": self.cmd.Quote(branchName),
 	}
-	return self.cmd.New(utils.ResolvePlaceholderString(branchLogCmdTemplate, templateValues)).DontLog()
+
+	resolvedTemplate := utils.ResolvePlaceholderString(branchLogCmdTemplate, templateValues)
+
+	return self.cmd.New(str.ToArgv(resolvedTemplate)).DontLog()
 }
 
 func (self *BranchCommands) SetCurrentBranchUpstream(remoteName string, remoteBranchName string) error {
-	cmdStr := NewGitCmd("branch").
-		Arg(fmt.Sprintf("--set-upstream-to=%s/%s", self.cmd.Quote(remoteName), self.cmd.Quote(remoteBranchName))).
-		ToString()
+	cmdArgs := NewGitCmd("branch").
+		Arg(fmt.Sprintf("--set-upstream-to=%s/%s", remoteName, remoteBranchName)).
+		ToArgv()
 
-	return self.cmd.New(cmdStr).Run()
+	return self.cmd.New(cmdArgs).Run()
 }
 
 func (self *BranchCommands) SetUpstream(remoteName string, remoteBranchName string, branchName string) error {
-	cmdStr := NewGitCmd("branch").
-		Arg(fmt.Sprintf("--set-upstream-to=%s/%s", self.cmd.Quote(remoteName), self.cmd.Quote(remoteBranchName))).
-		Arg(self.cmd.Quote(branchName)).
-		ToString()
+	cmdArgs := NewGitCmd("branch").
+		Arg(fmt.Sprintf("--set-upstream-to=%s/%s", remoteName, remoteBranchName)).
+		Arg(branchName).
+		ToArgv()
 
-	return self.cmd.New(cmdStr).Run()
+	return self.cmd.New(cmdArgs).Run()
 }
 
 func (self *BranchCommands) UnsetUpstream(branchName string) error {
-	cmdStr := NewGitCmd("branch").Arg("--unset-upstream", self.cmd.Quote(branchName)).
-		ToString()
+	cmdArgs := NewGitCmd("branch").Arg("--unset-upstream", branchName).
+		ToArgv()
 
-	return self.cmd.New(cmdStr).Run()
+	return self.cmd.New(cmdArgs).Run()
 }
 
 func (self *BranchCommands) GetCurrentBranchUpstreamDifferenceCount() (string, string) {
@@ -161,37 +165,37 @@ func (self *BranchCommands) GetCommitDifferences(from, to string) (string, strin
 }
 
 func (self *BranchCommands) countDifferences(from, to string) (string, error) {
-	cmdStr := NewGitCmd("rev-list").
+	cmdArgs := NewGitCmd("rev-list").
 		Arg(fmt.Sprintf("%s..%s", from, to)).
 		Arg("--count").
-		ToString()
+		ToArgv()
 
-	return self.cmd.New(cmdStr).DontLog().RunWithOutput()
+	return self.cmd.New(cmdArgs).DontLog().RunWithOutput()
 }
 
 func (self *BranchCommands) IsHeadDetached() bool {
-	cmdStr := NewGitCmd("symbolic-ref").Arg("-q", "HEAD").ToString()
+	cmdArgs := NewGitCmd("symbolic-ref").Arg("-q", "HEAD").ToArgv()
 
-	err := self.cmd.New(cmdStr).DontLog().Run()
+	err := self.cmd.New(cmdArgs).DontLog().Run()
 	return err != nil
 }
 
 func (self *BranchCommands) Rename(oldName string, newName string) error {
-	cmdStr := NewGitCmd("branch").
-		Arg("--move", self.cmd.Quote(oldName), self.cmd.Quote(newName)).
-		ToString()
+	cmdArgs := NewGitCmd("branch").
+		Arg("--move", oldName, newName).
+		ToArgv()
 
-	return self.cmd.New(cmdStr).Run()
+	return self.cmd.New(cmdArgs).Run()
 }
 
 func (self *BranchCommands) GetRawBranches() (string, error) {
-	cmdStr := NewGitCmd("for-each-ref").
+	cmdArgs := NewGitCmd("for-each-ref").
 		Arg("--sort=-committerdate").
-		Arg(`--format="%(HEAD)%00%(refname:short)%00%(upstream:short)%00%(upstream:track)"`).
+		Arg(`--format=%(HEAD)%00%(refname:short)%00%(upstream:short)%00%(upstream:track)`).
 		Arg("refs/heads").
-		ToString()
+		ToArgv()
 
-	return self.cmd.New(cmdStr).DontLog().RunWithOutput()
+	return self.cmd.New(cmdArgs).DontLog().RunWithOutput()
 }
 
 type MergeOpts struct {
@@ -199,16 +203,16 @@ type MergeOpts struct {
 }
 
 func (self *BranchCommands) Merge(branchName string, opts MergeOpts) error {
-	command := NewGitCmd("merge").
+	cmdArgs := NewGitCmd("merge").
 		Arg("--no-edit").
 		ArgIf(self.UserConfig.Git.Merging.Args != "", self.UserConfig.Git.Merging.Args).
 		ArgIf(opts.FastForwardOnly, "--ff-only").
-		Arg(self.cmd.Quote(branchName)).
-		ToString()
+		Arg(branchName).
+		ToArgv()
 
-	return self.cmd.New(command).Run()
+	return self.cmd.New(cmdArgs).Run()
 }
 
 func (self *BranchCommands) AllBranchesLogCmdObj() oscommands.ICmdObj {
-	return self.cmd.New(self.UserConfig.Git.AllBranchesLogCmd).DontLog()
+	return self.cmd.New(str.ToArgv(self.UserConfig.Git.AllBranchesLogCmd)).DontLog()
 }
diff --git a/pkg/commands/git_commands/branch_test.go b/pkg/commands/git_commands/branch_test.go
index 75c288203..dee2b03c8 100644
--- a/pkg/commands/git_commands/branch_test.go
+++ b/pkg/commands/git_commands/branch_test.go
@@ -21,21 +21,21 @@ func TestBranchGetCommitDifferences(t *testing.T) {
 		{
 			"Can't retrieve pushable count",
 			oscommands.NewFakeRunner(t).
-				Expect("git rev-list @{u}..HEAD --count", "", errors.New("error")),
+				ExpectGitArgs([]string{"rev-list", "@{u}..HEAD", "--count"}, "", errors.New("error")),
 			"?", "?",
 		},
 		{
 			"Can't retrieve pullable count",
 			oscommands.NewFakeRunner(t).
-				Expect("git rev-list @{u}..HEAD --count", "1\n", nil).
-				Expect("git rev-list HEAD..@{u} --count", "", errors.New("error")),
+				ExpectGitArgs([]string{"rev-list", "@{u}..HEAD", "--count"}, "1\n", nil).
+				ExpectGitArgs([]string{"rev-list", "HEAD..@{u}", "--count"}, "", errors.New("error")),
 			"?", "?",
 		},
 		{
 			"Retrieve pullable and pushable count",
 			oscommands.NewFakeRunner(t).
-				Expect("git rev-list @{u}..HEAD --count", "1\n", nil).
-				Expect("git rev-list HEAD..@{u} --count", "2\n", nil),
+				ExpectGitArgs([]string{"rev-list", "@{u}..HEAD", "--count"}, "1\n", nil).
+				ExpectGitArgs([]string{"rev-list", "HEAD..@{u}", "--count"}, "2\n", nil),
 			"1", "2",
 		},
 	}
@@ -54,7 +54,7 @@ func TestBranchGetCommitDifferences(t *testing.T) {
 
 func TestBranchNewBranch(t *testing.T) {
 	runner := oscommands.NewFakeRunner(t).
-		Expect(`git checkout -b "test" "refs/heads/master"`, "", nil)
+		ExpectGitArgs([]string{"checkout", "-b", "test", "refs/heads/master"}, "", nil)
 	instance := buildBranchCommands(commonDeps{runner: runner})
 
 	assert.NoError(t, instance.New("test", "refs/heads/master"))
@@ -73,7 +73,7 @@ func TestBranchDeleteBranch(t *testing.T) {
 		{
 			"Delete a branch",
 			false,
-			oscommands.NewFakeRunner(t).Expect(`git branch -d "test"`, "", nil),
+			oscommands.NewFakeRunner(t).ExpectGitArgs([]string{"branch", "-d", "test"}, "", nil),
 			func(err error) {
 				assert.NoError(t, err)
 			},
@@ -81,7 +81,7 @@ func TestBranchDeleteBranch(t *testing.T) {
 		{
 			"Force delete a branch",
 			true,
-			oscommands.NewFakeRunner(t).Expect(`git branch -D "test"`, "", nil),
+			oscommands.NewFakeRunner(t).ExpectGitArgs([]string{"branch", "-D", "test"}, "", nil),
 			func(err error) {
 				assert.NoError(t, err)
 			},
@@ -105,14 +105,14 @@ func TestBranchMerge(t *testing.T) {
 		userConfig *config.UserConfig
 		opts       MergeOpts
 		branchName string
-		expected   string
+		expected   []string
 	}{
 		{
 			testName:   "basic",
 			userConfig: &config.UserConfig{},
 			opts:       MergeOpts{},
 			branchName: "mybranch",
-			expected:   `git merge --no-edit "mybranch"`,
+			expected:   []string{"merge", "--no-edit", "mybranch"},
 		},
 		{
 			testName: "merging args",
@@ -125,14 +125,14 @@ func TestBranchMerge(t *testing.T) {
 			},
 			opts:       MergeOpts{},
 			branchName: "mybranch",
-			expected:   `git merge --no-edit --merging-args "mybranch"`,
+			expected:   []string{"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"`,
+			expected:   []string{"merge", "--no-edit", "--ff-only", "mybranch"},
 		},
 	}
 
@@ -140,7 +140,7 @@ func TestBranchMerge(t *testing.T) {
 		s := s
 		t.Run(s.testName, func(t *testing.T) {
 			runner := oscommands.NewFakeRunner(t).
-				Expect(s.expected, "", nil)
+				ExpectGitArgs(s.expected, "", nil)
 			instance := buildBranchCommands(commonDeps{runner: runner, userConfig: s.userConfig})
 
 			assert.NoError(t, instance.Merge(s.branchName, s.opts))
@@ -160,7 +160,7 @@ func TestBranchCheckout(t *testing.T) {
 	scenarios := []scenario{
 		{
 			"Checkout",
-			oscommands.NewFakeRunner(t).Expect(`git checkout "test"`, "", nil),
+			oscommands.NewFakeRunner(t).ExpectGitArgs([]string{"checkout", "test"}, "", nil),
 			func(err error) {
 				assert.NoError(t, err)
 			},
@@ -168,7 +168,7 @@ func TestBranchCheckout(t *testing.T) {
 		},
 		{
 			"Checkout forced",
-			oscommands.NewFakeRunner(t).Expect(`git checkout --force "test"`, "", nil),
+			oscommands.NewFakeRunner(t).ExpectGitArgs([]string{"checkout", "--force", "test"}, "", nil),
 			func(err error) {
 				assert.NoError(t, err)
 			},
@@ -214,7 +214,7 @@ func TestBranchCurrentBranchInfo(t *testing.T) {
 	scenarios := []scenario{
 		{
 			"says we are on the master branch if we are",
-			oscommands.NewFakeRunner(t).Expect(`git symbolic-ref --short HEAD`, "master", nil),
+			oscommands.NewFakeRunner(t).ExpectGitArgs([]string{"symbolic-ref", "--short", "HEAD"}, "master", nil),
 			func(info BranchInfo, err error) {
 				assert.NoError(t, err)
 				assert.EqualValues(t, "master", info.RefName)
@@ -225,8 +225,9 @@ func TestBranchCurrentBranchInfo(t *testing.T) {
 		{
 			"falls back to git `git branch --points-at=HEAD` if symbolic-ref fails",
 			oscommands.NewFakeRunner(t).
-				Expect(`git symbolic-ref --short HEAD`, "", errors.New("error")).
-				Expect(`git branch --points-at=HEAD --format="%(HEAD)%00%(objectname)%00%(refname)"`, "*\x006f71c57a8d4bd6c11399c3f55f42c815527a73a4\x00(HEAD detached at 6f71c57a)\n", nil),
+				ExpectGitArgs([]string{"symbolic-ref", "--short", "HEAD"}, "", errors.New("error")).
+				ExpectGitArgs([]string{"branch", "--points-at=HEAD", "--format=%(HEAD)%00%(objectname)%00%(refname)"},
+					"*\x006f71c57a8d4bd6c11399c3f55f42c815527a73a4\x00(HEAD detached at 6f71c57a)\n", nil),
 			func(info BranchInfo, err error) {
 				assert.NoError(t, err)
 				assert.EqualValues(t, "6f71c57a8d4bd6c11399c3f55f42c815527a73a4", info.RefName)
@@ -237,9 +238,9 @@ func TestBranchCurrentBranchInfo(t *testing.T) {
 		{
 			"handles a detached head (LANG=zh_CN.UTF-8)",
 			oscommands.NewFakeRunner(t).
-				Expect(`git symbolic-ref --short HEAD`, "", errors.New("error")).
-				Expect(
-					`git branch --points-at=HEAD --format="%(HEAD)%00%(objectname)%00%(refname)"`,
+				ExpectGitArgs([]string{"symbolic-ref", "--short", "HEAD"}, "", errors.New("error")).
+				ExpectGitArgs(
+					[]string{"branch", "--points-at=HEAD", "--format=%(HEAD)%00%(objectname)%00%(refname)"},
 					"*\x00679b0456f3db7c505b398def84e7d023e5b55a8d\x00（头指针在 679b0456 分离）\n"+
 						" \x00679b0456f3db7c505b398def84e7d023e5b55a8d\x00refs/heads/master\n",
 					nil),
@@ -253,8 +254,8 @@ func TestBranchCurrentBranchInfo(t *testing.T) {
 		{
 			"bubbles up error if there is one",
 			oscommands.NewFakeRunner(t).
-				Expect(`g