standardise how we read from the config

8 files changed, 240 insertions, 234 deletions

diff --git a/pkg/commands/config.go b/pkg/commands/config.go
index 4e747e32c..12bef33fd 100644
--- a/pkg/commands/config.go
+++ b/pkg/commands/config.go
@@ -43,10 +43,13 @@ func (c *GitCommand) colorArg() string {
 }
 
 func (c *GitCommand) GetConfigValue(key string) string {
-	output, err := c.OSCommand.RunCommandWithOutput("git config --get %s", key)
-	if err != nil {
-		// looks like this returns an error if there is no matching value which we're okay with
-		return ""
+	value, _ := c.getLocalGitConfig(key)
+	// we get an error if the key doesn't exist which we don't care about
+
+	if value != "" {
+		return value
 	}
-	return strings.TrimSpace(output)
+
+	value, _ = c.getGlobalGitConfig(key)
+	return value
 }
diff --git a/pkg/commands/files.go b/pkg/commands/files.go
index 05aa89978..93d1ccf03 100644
--- a/pkg/commands/files.go
+++ b/pkg/commands/files.go
@@ -2,6 +2,7 @@ package commands
 
 import (
 	"fmt"
+	"os/exec"
 	"path/filepath"
 	"strings"
 	"time"
@@ -9,6 +10,7 @@ import (
 	"github.com/go-errors/errors"
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/utils"
+	"github.com/mgutz/str"
 )
 
 // CatFile obtains the content of a file
@@ -262,3 +264,28 @@ func (c *GitCommand) ResetAndClean() error {
 
 	return c.RemoveUntrackedFiles()
 }
+
+// EditFile opens a file in a subprocess using whatever editor is available,
+// falling back to core.editor, VISUAL, EDITOR, then vi
+func (c *GitCommand) EditFile(filename string) (*exec.Cmd, error) {
+	editor, _ := c.getGlobalGitConfig("core.editor")
+
+	if editor == "" {
+		editor = c.OSCommand.Getenv("VISUAL")
+	}
+	if editor == "" {
+		editor = c.OSCommand.Getenv("EDITOR")
+	}
+	if editor == "" {
+		if err := c.OSCommand.RunCommand("which vi"); err == nil {
+			editor = "vi"
+		}
+	}
+	if editor == "" {
+		return nil, errors.New("No editor defined in $VISUAL, $EDITOR, or git config")
+	}
+
+	splitCmd := str.ToArgv(fmt.Sprintf("%s %s", editor, c.OSCommand.Quote(filename)))
+
+	return c.OSCommand.PrepareSubProcess(splitCmd[0], splitCmd[1:]...), nil
+}
diff --git a/pkg/commands/git_test.go b/pkg/commands/git_test.go
index e65871464..75bb5b8a6 100644
--- a/pkg/commands/git_test.go
+++ b/pkg/commands/git_test.go
@@ -2147,3 +2147,154 @@ func TestFindDotGitDir(t *testing.T) {
 		})
 	}
 }
+
+// TestEditFile is a function.
+func TestEditFile(t *testing.T) {
+	type scenario struct {
+		filename           string
+		command            func(string, ...string) *exec.Cmd
+		getenv             func(string) string
+		getGlobalGitConfig func(string) (string, error)
+		test               func(*exec.Cmd, error)
+	}
+
+	scenarios := []scenario{
+		{
+			"test",
+			func(name string, arg ...string) *exec.Cmd {
+				return exec.Command("exit", "1")
+			},
+			func(env string) string {
+				return ""
+			},
+			func(cf string) (string, error) {
+				return "", nil
+			},
+			func(cmd *exec.Cmd, err error) {
+				assert.EqualError(t, err, "No editor defined in $VISUAL, $EDITOR, or git config")
+			},
+		},
+		{
+			"test",
+			func(name string, arg ...string) *exec.Cmd {
+				if name == "which" {
+					return exec.Command("exit", "1")
+				}
+
+				assert.EqualValues(t, "nano", name)
+
+				return nil
+			},
+			func(env string) string {
+				return ""
+			},
+			func(cf string) (string, error) {
+				return "nano", nil
+			},
+			func(cmd *exec.Cmd, err error) {
+				assert.NoError(t, err)
+			},
+		},
+		{
+			"test",
+			func(name string, arg ...string) *exec.Cmd {
+				if name == "which" {
+					return exec.Command("exit", "1")
+				}
+
+				assert.EqualValues(t, "nano", name)
+
+				return nil
+			},
+			func(env string) string {
+				if env == "VISUAL" {
+					return "nano"
+				}
+
+				return ""
+			},
+			func(cf string) (string, error) {
+				return "", nil
+			},
+			func(cmd *exec.Cmd, err error) {
+				assert.NoError(t, err)
+			},
+		},
+		{
+			"test",
+			func(name string, arg ...string) *exec.Cmd {
+				if name == "which" {
+					return exec.Command("exit", "1")
+				}
+
+				assert.EqualValues(t, "emacs", name)
+
+				return nil
+			},
+			func(env string) string {
+				if env == "EDITOR" {
+					return "emacs"
+				}
+
+				return ""
+			},
+			func(cf string) (string, error) {
+				return "", nil
+			},
+			func(cmd *exec.Cmd, err error) {
+				assert.NoError(t, err)
+			},
+		},
+		{
+			"test",
+			func(name string, arg ...string) *exec.Cmd {
+				if name == "which" {
+					return exec.Command("echo")
+				}
+
+				assert.EqualValues(t, "vi", name)
+
+				return nil
+			},
+			func(env string) string {
+				return ""
+			},
+			func(cf string) (string, error) {
+				return "", nil
+			},
+			func(cmd *exec.Cmd, err error) {
+				assert.NoError(t, err)
+			},
+		},
+		{
+			"file/with space",
+			func(name string, args ...string) *exec.Cmd {
+				if name == "which" {
+					return exec.Command("echo")
+				}
+
+				assert.EqualValues(t, "vi", name)
+				assert.EqualValues(t, "file/with space", args[0])
+
+				return nil
+			},
+			func(env string) string {
+				return ""
+			},
+			func(cf string) (string, error) {
+				return "", nil
+			},
+			func(cmd *exec.Cmd, err error) {
+				assert.NoError(t, err)
+			},
+		},
+	}
+
+	for _, s := range scenarios {
+		gitCmd := NewDummyGitCommand()
+		gitCmd.OSCommand.Command = s.command
+		gitCmd.OSCommand.Getenv = s.getenv
+		gitCmd.getGlobalGitConfig = s.getGlobalGitConfig
+		s.test(gitCmd.EditFile(s.filename))
+	}
+}
diff --git a/pkg/commands/oscommands/os.go b/pkg/commands/oscommands/os.go
index da9fb311f..bbfcb8095 100644
--- a/pkg/commands/oscommands/os.go
+++ b/pkg/commands/oscommands/os.go
@@ -19,7 +19,6 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/utils"
 	"github.com/mgutz/str"
 	"github.com/sirupsen/logrus"
-	gitconfig "github.com/tcnksm/go-gitconfig"
 )
 
 // Platform stores the os state
@@ -36,25 +35,23 @@ type Platform struct {
 
 // OSCommand holds all the os commands
 type OSCommand struct {
-	Log                *logrus.Entry
-	Platform           *Platform
-	Config             config.AppConfigurer
-	Command            func(string, ...string) *exec.Cmd
-	BeforeExecuteCmd   func(*exec.Cmd)
-	GetGlobalGitConfig func(string) (string, error)
-	Getenv             func(string) string
+	Log              *logrus.Entry
+	Platform         *Platform
+	Config           config.AppConfigurer
+	Command          func(string, ...string) *exec.Cmd
+	BeforeExecuteCmd func(*exec.Cmd)
+	Getenv           func(string) string
 }
 
 // NewOSCommand os command runner
 func NewOSCommand(log *logrus.Entry, config config.AppConfigurer) *OSCommand {
 	return &OSCommand{
-		Log:                log,
-		Platform:           getPlatform(),
-		Config:             config,
-		Command:            exec.Command,
-		BeforeExecuteCmd:   func(*exec.Cmd) {},
-		GetGlobalGitConfig: gitconfig.Global,
-		Getenv:             os.Getenv,
+		Log:              log,
+		Platform:         getPlatform(),
+		Config:           config,
+		Command:          exec.Command,
+		BeforeExecuteCmd: func(*exec.Cmd) {},
+		Getenv:           os.Getenv,
 	}
 }
 
@@ -235,31 +232,6 @@ func (c *OSCommand) OpenLink(link string) error {
 	return err
 }
 
-// EditFile opens a file in a subprocess using whatever editor is available,
-// falling back to core.editor, VISUAL, EDITOR, then vi
-func (c *OSCommand) EditFile(filename string) (*exec.Cmd, error) {
-	editor, _ := c.GetGlobalGitConfig("core.editor")
-
-	if editor == "" {
-		editor = c.Getenv("VISUAL")
-	}
-	if editor == "" {
-		editor = c.Getenv("EDITOR")
-	}
-	if editor == "" {
-		if err := c.RunCommand("which vi"); err == nil {
-			editor = "vi"
-		}
-	}
-	if editor == "" {
-		return nil, errors.New("No editor defined in $VISUAL, $EDITOR, or git config")
-	}
-
-	splitCmd := str.ToArgv(fmt.Sprintf("%s %s", editor, c.Quote(filename)))
-
-	return c.PrepareSubProcess(splitCmd[0], splitCmd[1:]...), nil
-}
-
 // PrepareSubProcess iniPrepareSubProcessrocess then tells the Gui to switch to it
 // TODO: see if this needs to exist, given that ExecutableFromString does the same things
 func (c *OSCommand) PrepareSubProcess(cmdName string, commandArgs ...string) *exec.Cmd {
diff --git a/pkg/commands/oscommands/os_test.go b/pkg/commands/oscommands/os_test.go
index e4859484c..f8b3767bf 100644
--- a/pkg/commands/oscommands/os_test.go
+++ b/pkg/commands/oscommands/os_test.go
@@ -109,158 +109,6 @@ func TestOSCommandOpenFile(t *testing.T) {
 	}
 }
 
-// TestOSCommandEditFile is a function.
-func TestOSCommandEditFile(t *testing.T) {
-	type scenario struct {
-		filename           string
-		command            func(string, ...string) *exec.Cmd
-		getenv             func(string) string
-		getGlobalGitConfig func(string) (string, error)
-		test               func(*exec.Cmd, error)
-	}
-
-	scenarios := []scenario{
-		{
-			"test",
-			func(name string, arg ...string) *exec.Cmd {
-				return exec.Command("exit", "1")
-			},
-			func(env string) string {
-				return ""
-			},
-			func(cf string) (string, error) {
-				return "", nil
-			},
-			func(cmd *exec.Cmd, err error) {
-				assert.EqualError(t, err, "No editor defined in $VISUAL, $EDITOR, or git config")
-			},
-		},
-		{
-			"test",
-			func(name string, arg ...string) *exec.Cmd {
-				if name == "which" {
-					return exec.Command("exit", "1")
-				}
-
-				assert.EqualValues(t, "nano", name)
-
-				return nil
-			},
-			func(env string) string {
-				return ""
-			},
-			func(cf string) (string, error) {
-				return "nano", nil
-			},
-			func(cmd *exec.Cmd, err error) {
-				assert.NoError(t, err)
-			},
-		},
-		{
-			"test",
-			func(name string, arg ...string) *exec.Cmd {
-				if name == "which" {
-					return exec.Command("exit", "1")
-				}
-
-				assert.EqualValues(t, "nano", name)
-
-				return nil
-			},
-			func(env string) string {
-				if env == "VISUAL" {
-					return "nano"
-				}
-
-				return ""
-			},
-			func(cf string) (string, error) {
-				return "", nil
-			},
-			func(cmd *exec.Cmd, err error) {
-				assert.NoError(t, err)
-			},
-		},
-		{
-			"test",
-			func(name string, arg ...string) *exec.Cmd {
-				if name == "which" {
-					return exec.Command("exit", "1")
-				}
-
-				assert.EqualValues(t, "emacs", name)
-
-				return nil
-			},
-			func(env string) string {
-				if env == "EDITOR" {
-					return "emacs"
-				}
-
-				return ""
-			},
-			func(cf string) (string, error) {
-				return "", nil
-			},
-			func(cmd *exec.Cmd, err error) {
-				assert.NoError(t, err)
-			},
-		},
-		{
-			"test",
-			func(name string, arg ...string) *exec.Cmd {
-				if name == "which" {
-					return exec.Command("echo")
-				}
-
-				assert.EqualValues(t, "vi", name)
-
-				return nil
-			},
-			func(env string) string {
-				return ""
-			},
-			func(cf string) (string, error) {
-				return "", nil
-			},
-			func(cmd *exec.Cmd, err error) {
-				assert.NoError(t, err)
-			},
-		},
-		{
-			"file/with space",
-			func(name string, args ...string) *exec.Cmd {
-				if name == "which" {
-					return exec.Command("echo")
-				}
-
-				assert.EqualValues(t, "vi", name)
-				assert.EqualValues(t, "file/with space", args[0])
-
-				return nil
-			},
-			func(env string) string {
-				return ""
-			},
-			func(cf string) (string, error) {
-				return "", nil
-			},
-			func(cmd *exec.Cmd, err error) {
-				assert.NoError(t, err)
-			},
-		},
-	}
-
-	for _, s := range scenarios {
-		OSCmd := NewDummyOSCommand()
-		OSCmd.Command = s.command
-		OSCmd.GetGlobalGitConfig = s.getGlobalGitConfig
-		OSCmd.Getenv = s.getenv
-
-		s.test(OSCmd.EditFile(s.filename))
-	}
-}
-
 // TestOSCommandQuote is a function.
 func TestOSCommandQuote(t *testing.T) {
 	osCommand := NewDummyOSCommand()
diff --git a/pkg/commands/pull_request_test.go b/pkg/commands/pull_request_test.go
index 7ed02609a..110f5b58a 100644
--- a/pkg/commands/pull_request_test.go
+++ b/pkg/commands/pull_request_test.go
@@ -46,19 +46,21 @@ func TestGetRepoInfoFromURL(t *testing.T) {
 // TestCreatePullRequest is a function.
 func TestCreatePullRequest(t *testing.T) {
 	type scenario struct {
-		testName string
-		branch   *models.Branch
-		command  func(string, ...string) *exec.Cmd
-		test     func(err error)
+		testName  string
+		branch    *models.Branch
+		remoteUrl string
+		command   func(string, ...string) *exec.Cmd
+		test      func(err error)
 	}
 
 	scenarios := []scenario{
 		{
-			"Opens a link to new pull request on bitbucket",
-			&models.Branch{
+			testName: "Opens a link to new pull request on bitbucket",
+			branch: &models.Branch{
 				Name: "feature/profile-page",
 			},
-			func(cmd string, args ...string) *exec.Cmd {
+			remoteUrl: "git@bitbucket.org:johndoe/social_network.git",
+			command: func(cmd string, args ...string) *exec.Cmd {
 				// Handle git remote url call
 				if strings.HasPrefix(cmd, "git") {
 					return exec.Command("echo", "git@bitbucket.org:johndoe/social_network.git")
@@ -68,16 +70,17 @@ func TestCreatePullRequest(t *testing.T) {
 				assert.Equal(t, args, []string{"https://bitbucket.org/johndoe/social_network/pull-requests/new?source=feature/profile-page&t=1"})
 				return exec.Command("echo")
 			},
-			func(err error) {
+			test: func(err error) {
 				assert.NoError(t, err)
 			},
 		},
 		{
-			"Opens a link to new pull request on bitbucket with http remote url",
-			&models.Branch{
+			testName: "Opens a link to new pull request on bitbucket with http remote url",
+			branch: &models.Branch{
 				Name: "feature/events",
 			},
-			func(cmd string, args ...string) *exec.Cmd {
+			remoteUrl: "https://my_username@bitbucket.org/johndoe/social_network.git",
+			command: func(cmd string, args ...string) *exec.Cmd {
 				// Handle git remote url call
 				if strings.HasPrefix(cmd, "git") {
 					return exec.Command("echo", "https://my_username@bitbucket.org/johndoe/social_network.git")
@@ -87,16 +90,17 @@ func TestCreatePullRequest(t *testing.T) {
 				assert.Equal(t, args, []string{"https://bitbucket.org/johndoe/social_network/pull-requests/new?source=feature/events&t=1"})
 				return exec.Command("echo")
 			},
-			func(err error) {
+			test: func(err error) {
 				assert.NoError(t, err)
 			},
 		},
 		{
-			"Opens a link to new pull request on github",
-			&models.Branch{
+			testName: "Opens a link to new pull request on github",
+			branch: &models.Branch{
 				Name: "feature/sum-operation",
 			},
-			func(cmd string, args ...string) *exec.Cmd {
+			remoteUrl: "git@github.com:peter/calculator.git",
+			command: func(cmd string, args ...string) *exec.Cmd {
 				// Handle git remote url call
 				if strings.HasPrefix(cmd, "git") {
 					return exec.Command("echo", "git@github.com:peter/calculator.git")
@@ -106,16 +110,17 @@ func TestCreatePullRequest(t *testing.T) {
 				assert.Equal(t, args, []string{"https://github.com/peter/calculator/compare/feature/sum-operation?expand=1"})
 				return exec.Command("echo")
 			},
-			func(err error) {
+			test: func(err error) {
 				assert.NoError(t, err)
 			},
 		},
 		{
-			"Opens a link to new pull request on gitlab",
-			&models.Branch{
+			testName: "Opens a link to new pull request on gitlab",
+			branch: &models.Branch{
 				Name: "feature/ui",
 			},
-			func(cmd string, args ...string) *exec.Cmd {
+			remoteUrl: "git@gitlab.com:peter/calculator.git",
+			command: func(cmd string, args ...string) *exec.Cmd {
 				// Handle git remote url call
 				if strings.HasPrefix(cmd, "git") {
 					return exec.Command("echo", "git@gitlab.com:peter/calculator.git")
@@ -125,19 +130,20 @@ func TestCreatePullRequest(t *testing.T) {
 				assert.Equal(t, args, []string{"https://gitlab.com/peter/calculator/merge_requests/new?merge_request[source_branch]=feature/ui"})
 				return exec.Command("echo")
 			},
-			func(err error) {
+			test: func(err error) {
 				assert.NoError(t, err)
 			},
 		},
 		{
-			"Throws an error if git service is unsupported",
-			&models.Branch{
+			testName: "Throws an error if git service is unsupported",
+			branch: &models.Branch{
 				Name: "feature/divide-operation",
 			},
-			func(cmd string, args ...string) *exec.Cmd {
-				return exec.Command("echo", "git@something.com:peter/calculator.git")
+			remoteUrl: "git@something.com:peter/calculator.git",
+			command: func(cmd string, args ...string) *exec.Cmd {
+				return exec.Command("echo")
 			},
-			func(err error) {
+			test: func(err error) {
 				assert.Error(t, err)
 			},
 		},
@@ -155,6 +161,14 @@ func TestCreatePullRequest(t *testing.T) {
 				"invalid.work.com":   "noservice:invalid.work.com",
 				"noservice.work.com": "noservice.work.com",
 			}
+			gitCommand.getLocalGitConfig = func(path string) (string, error) {
+				assert.Equal(t, path, "remote.origin.url")
+				return s.remoteUrl, nil
+			}
+			gitCommand.getGlobalGitConfig = func(path string) (string, error) {
+				assert.Equal(t, path, "remote.origin.url")
+				return "", nil
+			}
 			dummyPullRequest := NewPullRequest(gitCommand)
 			s.test(dummyPullRequest.Create(s.branch))
 		})
diff --git a/pkg/commands/sync.go b/pkg/commands/sync.go
index 6f2c6207e..97a6e4748 100644
--- a/pkg/commands/sync.go
+++ b/pkg/commands/sync.go
@@ -13,10 +13,7 @@ func (c *GitCommand) usingGpg() bool {
 		return false
 	}
 
-	gpgsign, _ := c.getLocalGitConfig("commit.gpgsign")
-	if gpgsign == "" {
-		gpgsign, _ = c.getGlobalGitConfig("commit.gpgsign")
-	}
+	gpgsign := c.GetConfigValue("commit.gpgsign")
 	value := strings.ToLower(gpgsign)
 
 	return value == "true" || value == "1" || value == "yes" || value == "on"
@@ -25,14 +22,8 @@ func (c *GitCommand) usingGpg() bool {
 // Push pushes to a branch
 func (c *GitCommand) Push(branchName string, force bool, upstream string, args string, promptUserForCredential func(string) string) error {
 	followTagsFlag := "--follow-tags"
-	followsign, _ := c.getLocalGitConfig("push.followTags")
-	if followsign == "false" {
+	if c.GetConfigValue("push.followTags") == "false" {
 		followTagsFlag = ""
-	} else if followsign == "" {
-		followsign, _ = c.getGlobalGitConfig("push.followTags")
-		if followsign == "false" {
-			followTagsFlag = ""
-		}
 	}
 
 	forceFlag := ""
diff --git a/pkg/gui/files_panel.go b/pkg/gui/files_panel.go
index 26d7e1bb1..1d17b7771 100644
--- a/pkg/gui/files_panel.go
+++ b/pkg/gui/files_panel.go
@@ -417,7 +417,7 @@ func (gui *Gui) PrepareSubProcess(command string) {
 }
 
 func (gui *Gui) editFile(filename string) error {
-	_, err := gui.runSyncOrAsyncCommand(gui.OSCommand.EditFile(filename))
+	_, err := gui.runSyncOrAsyncCommand(gui.GitCommand.EditFile(filename))
 	return err
 }