From 999e170f1d17b652dad231f1cbde6ab4bbeae8c7 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Wed, 25 Nov 2020 08:52:00 +1100 Subject: [PATCH] standardise how we read from the config --- pkg/commands/config.go | 13 ++- pkg/commands/files.go | 27 +++++ pkg/commands/git_test.go | 151 ++++++++++++++++++++++++++++ pkg/commands/oscommands/os.go | 52 +++------- pkg/commands/oscommands/os_test.go | 152 ----------------------------- pkg/commands/pull_request_test.go | 64 +++++++----- pkg/commands/sync.go | 13 +-- pkg/gui/files_panel.go | 2 +- 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 }