Merge pull request #2523 from stefanhaller/editor-config

5 files changed, 309 insertions, 17 deletions

diff --git a/pkg/commands/git_commands/file.go b/pkg/commands/git_commands/file.go
index 111733bb7..94563be65 100644
--- a/pkg/commands/git_commands/file.go
+++ b/pkg/commands/git_commands/file.go
@@ -3,8 +3,10 @@ package git_commands
 import (
 	"os"
 	"strconv"
+	"strings"
 
 	"github.com/go-errors/errors"
+	"github.com/jesseduffield/lazygit/pkg/config"
 	"github.com/jesseduffield/lazygit/pkg/utils"
 )
 
@@ -27,7 +29,7 @@ func (self *FileCommands) Cat(fileName string) (string, error) {
 	return string(buf), nil
 }
 
-func (self *FileCommands) GetEditCmdStr(filename string, lineNumber int) (string, error) {
+func (self *FileCommands) GetEditCmdStrLegacy(filename string, lineNumber int) (string, error) {
 	editor := self.UserConfig.OS.EditCommand
 
 	if editor == "" {
@@ -72,3 +74,82 @@ func (self *FileCommands) GetEditCmdStr(filename string, lineNumber int) (string
 	}
 	return utils.ResolvePlaceholderString(editCmdTemplate, templateValues), nil
 }
+
+func (self *FileCommands) GetEditCmdStr(filename string) (string, bool) {
+	// Legacy support for old config; to be removed at some point
+	if self.UserConfig.OS.Edit == "" && self.UserConfig.OS.EditCommandTemplate != "" {
+		if cmdStr, err := self.GetEditCmdStrLegacy(filename, 1); err == nil {
+			return cmdStr, true
+		}
+	}
+
+	template, editInTerminal := config.GetEditTemplate(&self.UserConfig.OS, self.guessDefaultEditor)
+
+	templateValues := map[string]string{
+		"filename": self.cmd.Quote(filename),
+	}
+
+	cmdStr := utils.ResolvePlaceholderString(template, templateValues)
+	return cmdStr, editInTerminal
+}
+
+func (self *FileCommands) GetEditAtLineCmdStr(filename string, lineNumber int) (string, bool) {
+	// Legacy support for old config; to be removed at some point
+	if self.UserConfig.OS.EditAtLine == "" && self.UserConfig.OS.EditCommandTemplate != "" {
+		if cmdStr, err := self.GetEditCmdStrLegacy(filename, lineNumber); err == nil {
+			return cmdStr, true
+		}
+	}
+
+	template, editInTerminal := config.GetEditAtLineTemplate(&self.UserConfig.OS, self.guessDefaultEditor)
+
+	templateValues := map[string]string{
+		"filename": self.cmd.Quote(filename),
+		"line":     strconv.Itoa(lineNumber),
+	}
+
+	cmdStr := utils.ResolvePlaceholderString(template, templateValues)
+	return cmdStr, editInTerminal
+}
+
+func (self *FileCommands) GetEditAtLineAndWaitCmdStr(filename string, lineNumber int) string {
+	// Legacy support for old config; to be removed at some point
+	if self.UserConfig.OS.EditAtLineAndWait == "" && self.UserConfig.OS.EditCommandTemplate != "" {
+		if cmdStr, err := self.GetEditCmdStrLegacy(filename, lineNumber); err == nil {
+			return cmdStr
+		}
+	}
+
+	template := config.GetEditAtLineAndWaitTemplate(&self.UserConfig.OS, self.guessDefaultEditor)
+
+	templateValues := map[string]string{
+		"filename": self.cmd.Quote(filename),
+		"line":     strconv.Itoa(lineNumber),
+	}
+
+	cmdStr := utils.ResolvePlaceholderString(template, templateValues)
+	return cmdStr
+}
+
+func (self *FileCommands) guessDefaultEditor() string {
+	// Try to query a few places where editors get configured
+	editor := self.config.GetCoreEditor()
+	if editor == "" {
+		editor = self.os.Getenv("GIT_EDITOR")
+	}
+	if editor == "" {
+		editor = self.os.Getenv("VISUAL")
+	}
+	if editor == "" {
+		editor = self.os.Getenv("EDITOR")
+	}
+
+	if editor != "" {
+		// At this point, it might be more than just the name of the editor;
+		// e.g. it might be "code -w" or "vim -u myvim.rc". So assume that
+		// everything up to the first space is the editor name.
+		editor = strings.Split(editor, " ")[0]
+	}
+
+	return editor
+}
diff --git a/pkg/commands/git_commands/file_test.go b/pkg/commands/git_commands/file_test.go
index 4b9128dfe..2367e4fcb 100644
--- a/pkg/commands/git_commands/file_test.go
+++ b/pkg/commands/git_commands/file_test.go
@@ -10,7 +10,7 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestEditFileCmdStr(t *testing.T) {
+func TestEditFileCmdStrLegacy(t *testing.T) {
 	type scenario struct {
 		filename                  string
 		configEditCommand         string
@@ -172,7 +172,206 @@ func TestEditFileCmdStr(t *testing.T) {
 			getenv:     s.getenv,
 		})
 
-		s.test(instance.GetEditCmdStr(s.filename, 1))
+		s.test(instance.GetEditCmdStrLegacy(s.filename, 1))
 		s.runner.CheckForMissingCalls()
 	}
 }
+
+func TestEditFileCmd(t *testing.T) {
+	type scenario struct {
+		filename               string
+		osConfig               config.OSConfig
+		expectedCmdStr         string
+		expectedEditInTerminal bool
+	}
+
+	scenarios := []scenario{
+		{
+			filename:               "test",
+			osConfig:               config.OSConfig{},
+			expectedCmdStr:         `vim -- "test"`,
+			expectedEditInTerminal: true,
+		},
+		{
+			filename: "test",
+			osConfig: config.OSConfig{
+				Edit: "nano {{filename}}",
+			},
+			expectedCmdStr:         `nano "test"`,
+			expectedEditInTerminal: true,
+		},
+		{
+			filename: "file/with space",
+			osConfig: config.OSConfig{
+				EditPreset: "sublime",
+			},
+			expectedCmdStr:         `subl -- "file/with space"`,
+			expectedEditInTerminal: false,
+		},
+	}
+
+	for _, s := range scenarios {
+		userConfig := config.GetDefaultConfig()
+		userConfig.OS = s.osConfig
+
+		instance := buildFileCommands(commonDeps{
+			userConfig: userConfig,
+		})
+
+		cmdStr, editInTerminal := instance.GetEditCmdStr(s.filename)
+		assert.Equal(t, s.expectedCmdStr, cmdStr)
+		assert.Equal(t, s.expectedEditInTerminal, editInTerminal)
+	}
+}
+
+func TestEditFileAtLineCmd(t *testing.T) {
+	type scenario struct {
+		filename               string
+		lineNumber             int
+		osConfig               config.OSConfig
+		expectedCmdStr         string
+		expectedEditInTerminal bool
+	}
+
+	scenarios := []scenario{
+		{
+			filename:               "test",
+			lineNumber:             42,
+			osConfig:               config.OSConfig{},
+			expectedCmdStr:         `vim +42 -- "test"`,
+			expectedEditInTerminal: true,
+		},
+		{
+			filename:   "test",
+			lineNumber: 35,
+			osConfig: config.OSConfig{
+				EditAtLine: "nano +{{line}} {{filename}}",
+			},
+			expectedCmdStr:         `nano +35 "test"`,
+			expectedEditInTerminal: true,
+		},
+		{
+			filename:   "file/with space",
+			lineNumber: 12,
+			osConfig: config.OSConfig{
+				EditPreset: "sublime",
+			},
+			expectedCmdStr:         `subl -- "file/with space":12`,
+			expectedEditInTerminal: false,
+		},
+	}
+
+	for _, s := range scenarios {
+		userConfig := config.GetDefaultConfig()
+		userConfig.OS = s.osConfig
+
+		instance := buildFileCommands(commonDeps{
+			userConfig: userConfig,
+		})
+
+		cmdStr, editInTerminal := instance.GetEditAtLineCmdStr(s.filename, s.lineNumber)
+		assert.Equal(t, s.expectedCmdStr, cmdStr)
+		assert.Equal(t, s.expectedEditInTerminal, editInTerminal)
+	}
+}
+
+func TestEditFileAtLineAndWaitCmd(t *testing.T) {
+	type scenario struct {
+		filename       string
+		lineNumber     int
+		osConfig       config.OSConfig
+		expectedCmdStr string
+	}
+
+	scenarios := []scenario{
+		{
+			filename:       "test",
+			lineNumber:     42,
+			osConfig:       config.OSConfig{},
+			expectedCmdStr: `vim +42 -- "test"`,
+		},
+		{
+			filename:   "file/with space",
+			lineNumber: 12,
+			osConfig: config.OSConfig{
+				EditPreset: "sublime",
+			},
+			expectedCmdStr: `subl --wait -- "file/with space":12`,
+		},
+	}
+
+	for _, s := range scenarios {
+		userConfig := config.GetDefaultConfig()
+		userConfig.OS = s.osConfig
+
+		instance := buildFileCommands(commonDeps{
+			userConfig: userConfig,
+		})
+
+		cmdStr := instance.GetEditAtLineAndWaitCmdStr(s.filename, s.lineNumber)
+		assert.Equal(t, s.expectedCmdStr, cmdStr)
+	}
+}
+
+func TestGuessDefaultEditor(t *testing.T) {
+	type scenario struct {
+		gitConfigMockResponses map[string]string
+		getenv                 func(string) string
+		expectedResult         string
+	}
+
+	scenarios := []scenario{
+		{
+			gitConfigMockResponses: nil,
+			getenv: func(env string) string {
+				return ""
+			},
+			expectedResult: "",
+		},
+		{
+			gitConfigMockResponses: map[string]string{"core.editor": "nano"},
+			getenv: func(env string) string {
+				return ""
+			},
+			expectedResult: "nano",
+		},
+		{
+			gitConfigMockResponses: map[string]string{"core.editor": "code -w"},
+			getenv: func(env string) string {
+				return ""
+			},
+			expectedResult: "code",
+		},
+		{
+			gitConfigMockResponses: nil,
+			getenv: func(env string) string {
+				if env == "VISUAL" {
+					return "emacs"
+				}
+
+				return ""
+			},
+			expectedResult: "emacs",
+		},
+		{
+			gitConfigMockResponses: nil,
+			getenv: func(env string) string {
+				if env == "EDITOR" {
+					return "bbedit -w"
+				}
+
+				return ""
+			},
+			expectedResult: "bbedit",
+		},
+	}
+
+	for _, s := range scenarios {
+		instance := buildFileCommands(commonDeps{
+			gitConfig: git_config.NewFakeGitConfig(s.gitConfigMockResponses),
+			getenv:    s.getenv,
+		})
+
+		assert.Equal(t, s.expectedResult, instance.guessDefaultEditor())
+	}
+}
diff --git a/pkg/commands/oscommands/os.go b/pkg/commands/oscommands/os.go
index 39149ce84..c8f38843b 100644
--- a/pkg/commands/oscommands/os.go
+++ b/pkg/commands/oscommands/os.go
@@ -78,21 +78,30 @@ func FileType(path string) string {
 }
 
 func (c *OSCommand) OpenFile(filename string) error {
-	return c.OpenFileAtLine(filename, 1)
-}
-
-func (c *OSCommand) OpenFileAtLine(filename string, lineNumber int) error {
-	commandTemplate := c.UserConfig.OS.OpenCommand
+	commandTemplate := c.UserConfig.OS.Open
+	if commandTemplate == "" {
+		// Legacy support
+		commandTemplate = c.UserConfig.OS.OpenCommand
+	}
+	if commandTemplate == "" {
+		commandTemplate = config.GetPlatformDefaultConfig().Open
+	}
 	templateValues := map[string]string{
 		"filename": c.Quote(filename),
-		"line":     fmt.Sprintf("%d", lineNumber),
 	}
 	command := utils.ResolvePlaceholderString(commandTemplate, templateValues)
 	return c.Cmd.NewShell(command).Run()
 }
 
 func (c *OSCommand) OpenLink(link string) error {
-	commandTemplate := c.UserConfig.OS.OpenLinkCommand
+	commandTemplate := c.UserConfig.OS.OpenLink
+	if commandTemplate == "" {
+		// Legacy support
+		commandTemplate = c.UserConfig.OS.OpenLinkCommand
+	}
+	if commandTemplate == "" {
+		commandTemplate = config.GetPlatformDefaultConfig().OpenLink
+	}
 	templateValues := map[string]string{
 		"link": c.Quote(link),
 	}
diff --git a/pkg/commands/oscommands/os_test_default.go b/pkg/commands/oscommands/os_default_test.go
index 39a1226d2..1f534f6d3 100644
--- a/pkg/commands/oscommands/os_test_default.go
+++ b/pkg/commands/oscommands/os_default_test.go
@@ -75,7 +75,7 @@ func TestOSCommandOpenFileDarwin(t *testing.T) {
 	for _, s := range scenarios {
 		oSCmd := NewDummyOSCommandWithRunner(s.runner)
 		oSCmd.Platform.OS = "darwin"
-		oSCmd.UserConfig.OS.OpenCommand = "open {{filename}}"
+		oSCmd.UserConfig.OS.Open = "open {{filename}}"
 
 		s.test(oSCmd.OpenFile(s.filename))
 	}
@@ -135,7 +135,7 @@ func TestOSCommandOpenFileLinux(t *testing.T) {
 	for _, s := range scenarios {
 		oSCmd := NewDummyOSCommandWithRunner(s.runner)
 		oSCmd.Platform.OS = "linux"
-		oSCmd.UserConfig.OS.OpenCommand = `xdg-open {{filename}} > /dev/null`
+		oSCmd.UserConfig.OS.Open = `xdg-open {{filename}} > /dev/null`
 
 		s.test(oSCmd.OpenFile(s.filename))
 	}
diff --git a/pkg/commands/oscommands/os_test_windows.go b/pkg/commands/oscommands/os_windows_test.go
index cf9c1e68a..acb971b59 100644
--- a/pkg/commands/oscommands/os_test_windows.go
+++ b/pkg/commands/oscommands/os_windows_test.go
@@ -6,6 +6,7 @@ package oscommands
 import (
 	"testing"
 
+	"github.com/cli/safeexec"
 	"github.com/go-errors/errors"
 	"github.com/stretchr/testify/assert"
 )
@@ -19,11 +20,13 @@ func TestOSCommandOpenFileWindows(t *testing.T) {
 		test     func(error)
 	}
 
+	fullCmdPath, _ := safeexec.LookPath("cmd")
+
 	scenarios := []scenario{
 		{
 			filename: "test",
 			runner: NewFakeRunner(t).
-				ExpectArgs([]string{"cmd", "/c", "start", "", "test"}, "", errors.New("error")),
+				ExpectArgs([]string{fullCmdPath, "/c", "start", "", "test"}, "", errors.New("error")),
 			test: func(err error) {
 				assert.Error(t, err)
 			},
@@ -31,7 +34,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) {
 		{
 			filename: "test",
 			runner: NewFakeRunner(t).
-				ExpectArgs([]string{"cmd", "/c", "start", "", "test"}, "", nil),
+				ExpectArgs([]string{fullCmdPath, "/c", "start", "", "test"}, "", nil),
 			test: func(err error) {
 				assert.NoError(t, err)
 			},
@@ -39,7 +42,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) {
 		{
 			filename: "filename with spaces",
 			runner: NewFakeRunner(t).
-				ExpectArgs([]string{"cmd", "/c", "start", "", "filename with spaces"}, "", nil),
+				ExpectArgs([]string{fullCmdPath, "/c", "start", "", "filename with spaces"}, "", nil),
 			test: func(err error) {
 				assert.NoError(t, err)
 			},
@@ -47,7 +50,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) {
 		{
 			filename: "let's_test_with_single_quote",
 			runner: NewFakeRunner(t).
-				ExpectArgs([]string{"cmd", "/c", "start", "", "let's_test_with_single_quote"}, "", nil),
+				ExpectArgs([]string{fullCmdPath, "/c", "start", "", "let's_test_with_single_quote"}, "", nil),
 			test: func(err error) {
 				assert.NoError(t, err)
 			},
@@ -55,7 +58,7 @@ func TestOSCommandOpenFileWindows(t *testing.T) {
 		{
 			filename: "$USER.txt",
 			runner: NewFakeRunner(t).
-				ExpectArgs([]string{"cmd", "/c", "start", "", "$USER.txt"}, "", nil),
+				ExpectArgs([]string{fullCmdPath, "/c", "start", "", "$USER.txt"}, "", nil),
 			test: func(err error) {
 				assert.NoError(t, err)
 			},