Implement edit presets

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,61 @@ func (self *FileCommands) GetEditCmdStr(filename string, lineNumber int) (string
 	}
 	return utils.ResolvePlaceholderString(editCmdTemplate, templateValues), nil
 }
+
+func (self *FileCommands) GetEditCmdStr(filename string) (string, bool) {
+	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) {
+	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 {
+	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
+}

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())
+	}
+}

pkg/config/config_default_platform.go

@@ -6,9 +6,7 @@ package config
 // GetPlatformDefaultConfig gets the defaults for the platform
 func GetPlatformDefaultConfig() OSConfig {
 	return OSConfig{
-		EditCommand:         ``,
-		EditCommandTemplate: "",
-		OpenCommand:         "open -- {{filename}}",
-		OpenLinkCommand:     "open {{link}}",
+		OpenCommand:     "open -- {{filename}}",
+		OpenLinkCommand: "open {{link}}",
 	}
 }

pkg/config/config_linux.go

@@ -29,17 +29,13 @@ func isContainer() bool {
 func GetPlatformDefaultConfig() OSConfig {
 	if isWSL() && !isContainer() {
 		return OSConfig{
-			EditCommand:         ``,
-			EditCommandTemplate: "",
-			OpenCommand:         `powershell.exe start explorer.exe {{filename}} >/dev/null`,
-			OpenLinkCommand:     `powershell.exe start {{link}} >/dev/null`,
+			OpenCommand:     `powershell.exe start explorer.exe {{filename}} >/dev/null`,
+			OpenLinkCommand: `powershell.exe start {{link}} >/dev/null`,
 		}
 	}
 
 	return OSConfig{
-		EditCommand:         ``,
-		EditCommandTemplate: "",
-		OpenCommand:         `xdg-open {{filename}} >/dev/null`,
-		OpenLinkCommand:     `xdg-open {{link}} >/dev/null`,
+		OpenCommand:     `xdg-open {{filename}} >/dev/null`,
+		OpenLinkCommand: `xdg-open {{link}} >/dev/null`,
 	}
 }

pkg/config/config_windows.go

@@ -3,9 +3,7 @@ package config
 // GetPlatformDefaultConfig gets the defaults for the platform
 func GetPlatformDefaultConfig() OSConfig {
 	return OSConfig{
-		EditCommand:         ``,
-		EditCommandTemplate: "",
-		OpenCommand:         `start "" {{filename}}`,
-		OpenLinkCommand:     `start "" {{link}}`,
+		OpenCommand:     `start "" {{filename}}`,
+		OpenLinkCommand: `start "" {{link}}`,
 	}
 }

pkg/config/editor_presets.go

@@ -0,0 +1,109 @@
+package config
+
+func GetEditTemplate(osConfig *OSConfig, guessDefaultEditor func() string) (string, bool) {
+	preset := getPreset(osConfig, guessDefaultEditor)
+	template := osConfig.Edit
+	if template == "" {
+		template = preset.editTemplate
+	}
+
+	return template, getEditInTerminal(osConfig, preset)
+}
+
+func GetEditAtLineTemplate(osConfig *OSConfig, guessDefaultEditor func() string) (string, bool) {
+	preset := getPreset(osConfig, guessDefaultEditor)
+	template := osConfig.EditAtLine
+	if template == "" {
+		template = preset.editAtLineTemplate
+	}
+	return template, getEditInTerminal(osConfig, preset)
+}
+
+func GetEditAtLineAndWaitTemplate(osConfig *OSConfig, guessDefaultEditor func() string) string {
+	preset := getPreset(osConfig, guessDefaultEditor)
+	template := osConfig.EditAtLineAndWait
+	if template == "" {
+		template = preset.editAtLineAndWaitTemplate
+	}
+	return template
+}
+
+type editPreset struct {
+	editTemplate              string
+	editAtLineTemplate        string
+	editAtLineAndWaitTemplate string
+	editInTerminal            bool
+}
+
+func getPreset(osConfig *OSConfig, guessDefaultEditor func() string) *editPreset {
+	presets := map[string]*editPreset{
+		"vi":    standardTerminalEditorPreset("vi"),
+		"vim":   standardTerminalEditorPreset("vim"),
+		"nvim":  standardTerminalEditorPreset("nvim"),
+		"emacs": standardTerminalEditorPreset("emacs"),
+		"nano":  standardTerminalEditorPreset("nano"),
+		"vscode": {
+			editTemplate:              "code --reuse-window -- {{filename}}",
+			editAtLineTemplate:        "code --reuse-window --goto -- {{filename}}:{{line}}",
+			editAtLineAndWaitTemplate: "code --reuse-window --goto --wait -- {{filename}}:{{line}}",
+			editInTerminal:            false,
+		},
+		"sublime": {
+			editTemplate:              "subl -- {{filename}}",
+			editAtLineTemplate:        "subl -- {{filename}}:{{line}}",
+			editAtLineAndWaitTemplate: "subl --wait -- {{filename}}:{{line}}",
+			editInTerminal:            false,
+		},
+		"bbedit": {
+			editTemplate:              "bbedit -- {{filename}}",
+			editAtLineTemplate:        "bbedit +{{line}} -- {{filename}}",
+			editAtLineAndWaitTemplate: "bbedit +{{line}} --wait -- {{filename}}",
+			editInTerminal:            false,
+		},
+		"xcode": {
+			editTemplate:              "xed -- {{filename}}",
+			editAtLineTemplate:        "xed --line {{line}} -- {{filename}}",
+			editAtLineAndWaitTemplate: "xed --line {{line}} --wait -- {{filename}}",
+			editInTerminal:            false,
+		},
+	}
+
+	// Some of our presets have a different name than the editor they are using.
+	editorToPreset := map[string]string{
+		"code": "vscode",
+		"subl": "sublime",
+		"xed":  "xcode",
+	}
+
+	presetName := osConfig.EditPreset
+	if presetName == "" {
+		defaultEditor := guessDefaultEditor()
+		if presets[defaultEditor] != nil {
+			presetName = defaultEditor
+		} else if p := editorToPreset[defaultEditor]; p != "" {
+			presetName = p
+		}
+	}
+
+	if presetName == "" || presets[presetName] == nil {
+		presetName = "vim"
+	}
+
+	return presets[presetName]
+}
+
+func standardTerminalEditorPreset(editor string) *editPreset {
+	return &editPreset{
+		editTemplate:              editor + " -- {{filename}}",
+		editAtLineTemplate:        editor + " +{{line}} -- {{filename}}",
+		editAtLineAndWaitTemplate: editor + " +{{line}} -- {{filename}}",
+		editInTerminal:            true,
+	}
+}
+
+func getEditInTerminal(osConfig *OSConfig, preset *editPreset) bool {
+	if osConfig.EditInTerminal != nil {
+		return *osConfig.EditInTerminal
+	}
+	return preset.editInTerminal
+}

pkg/config/editor_presets_test.go

@@ -0,0 +1,126 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetEditTemplate(t *testing.T) {
+	trueVal := true
+
+	scenarios := []struct {
+		name                              string
+		osConfig                          *OSConfig
+		guessDefaultEditor                func() string
+		expectedEditTemplate              string
+		expectedEditAtLineTemplate        string
+		expectedEditAtLineAndWaitTemplate string
+		expectedEditInTerminal            bool
+	}{
+		{
+			"Default template is vim",
+			&OSConfig{},
+			func() string { return "" },
+			"vim -- {{filename}}",
+			"vim +{{line}} -- {{filename}}",
+			"vim +{{line}} -- {{filename}}",
+			true,
+		},
+		{
+			"Setting a preset",
+			&OSConfig{
+				EditPreset: "vscode",
+			},
+			func() string { return "" },
+			"code --reuse-window -- {{filename}}",
+			"code --reuse-window --goto -- {{filename}}:{{line}}",
+			"code --reuse-window --goto --wait -- {{filename}}:{{line}}",
+			false,
+		},
+		{
+			"Setting a preset wins over guessed editor",
+			&OSConfig{
+				EditPreset: "vscode",
+			},
+			func() string { return "nano" },
+			"code --reuse-window -- {{filename}}",
+			"code --reuse-window --goto -- {{filename}}:{{line}}",
+			"code --reuse-window --goto --wait -- {{filename}}:{{line}}",
+			false,
+		},
+		{
+			"Overriding a preset with explicit config (edit)",
+			&OSConfig{
+				EditPreset:     "vscode",
+				Edit:           "myeditor {{filename}}",
+				EditInTerminal: &trueVal,
+			},
+			func() string { return "" },
+			"myeditor {{filename}}",
+			"code --reuse-window --goto -- {{filename}}:{{line}}",
+			"code --reuse-window --goto --wait -- {{filename}}:{{line}}",
+			true,
+		},
+		{
+			"Overriding a preset with explicit config (edit at line)",
+			&OSConfig{
+				EditPreset:     "vscode",
+				EditAtLine:     "myeditor --line={{line}} {{filename}}",
+				EditInTerminal: &trueVal,
+			},
+			func() string { return "" },
+			"code --reuse-window -- {{filename}}",
+			"myeditor --line={{line}} {{filename}}",
+			"code --reuse-window --goto --wait -- {{filename}}:{{line}}",
+			true,
+		},
+		{
+			"Overriding a preset with explicit config (edit at line and wait)",
+			&OSConfig{
+				EditPreset:        "vscode",
+				EditAtLineAndWait: "myeditor --line={{line}} -w {{filename}}",
+				EditInTerminal:    &trueVal,
+			},
+			func() string { return "" },
+			"code --reuse-window -- {{filename}}",
+			"code --reuse-window --goto -- {{filename}}:{{line}}",
+			"myeditor --line={{line}} -w {{filename}}",
+			true,
+		},
+		{
+			"Unknown preset name",
+			&OSConfig{
+				EditPreset: "thisPresetDoesNotExist",
+			},
+			func() string { return "" },
+			"vim -- {{filename}}",
+			"vim +{{line}} -- {{filename}}",
+			"vim +{{line}} -- {{filename}}",
+			true,
+		},
+		{
+			"Guessing a preset from guessed editor",
+			&OSConfig{},
+			func() string { return "emacs" },
+			"emacs -- {{filename}}",
+			"emacs +{{line}} -- {{filename}}",
+			"emacs +{{line}} -- {{filename}}",
+			true,
+		},
+	}
+	for _, s := range scenarios {
+		t.Run(s.name, func(t *testing.T) {
+			template, editInTerminal := GetEditTemplate(s.osConfig, s.guessDefaultEditor)
+			assert.Equal(t, s.expectedEditTemplate, template)
+			assert.Equal(t, s.expectedEditInTerminal, editInTerminal)
+
+			template, editInTerminal = GetEditAtLineTemplate(s.osConfig, s.guessDefaultEditor)
+			assert.Equal(t, s.expectedEditAtLineTemplate, template)
+			assert.Equal(t, s.expectedEditInTerminal, editInTerminal)
+
+			template = GetEditAtLineAndWaitTemplate(s.osConfig, s.guessDefaultEditor)
+			assert.Equal(t, s.expectedEditAtLineAndWaitTemplate, template)
+		})
+	}
+}

pkg/config/user_config.go

@@ -291,10 +291,39 @@ type KeybindingSubmodulesConfig struct {
 
 // OSConfig contains config on the level of the os
 type OSConfig struct {
-	// EditCommand is the command for editing a file
+	// Command for editing a file. Should contain "{{filename}}".
+	Edit string `yaml:"edit,omitempty"`
+
+	// Command for editing a file at a given line number. Should contain
+	// "{{filename}}", and may optionally contain "{{line}}".
+	EditAtLine string `yaml:"editAtLine,omitempty"`
+
+	// Same as EditAtLine, except that the command needs to wait until the
+	// window is closed.
+	EditAtLineAndWait string `yaml:"editAtLineAndWait,omitempty"`
+
+	// Whether the given edit commands use the terminal. Used to decide whether
+	// lazygit needs to suspend to the background before calling the editor.
+	// Pointer to bool so that we can distinguish unset (nil) from false.
+	EditInTerminal *bool `yaml:"editInTerminal,omitempty"`
+
+	// A built-in preset that sets all of the above settings. Supported presets
+	// are defined in the getPreset function in editor_presets.go.
+	EditPreset string `yaml:"editPreset,omitempty"`
+
+	// --------
+
+	// The following configs are all deprecated and kept for backward
+	// compatibility. They will be removed in the future.
+
+	// EditCommand is the command for editing a file.
+	// Deprecated: use Edit instead. Note that semantics are different:
+	// EditCommand is just the command itself, whereas Edit contains a
+	// "{{filename}}" variable.
 	EditCommand string `yaml:"editCommand,omitempty"`
 
 	// EditCommandTemplate is the command template for editing a file
+	// Deprecated: use EditAtLine instead.
 	EditCommandTemplate string `yaml:"editCommandTemplate,omitempty"`
 
 	// OpenCommand is the command for opening a file

pkg/gui/controllers/helpers/files_helper.go

@@ -34,19 +34,32 @@ func NewFilesHelper(
 var _ IFilesHelper = &FilesHelper{}
 
 func (self *FilesHelper) EditFile(filename string) error {
-	return self.EditFileAtLine(filename, 1)
+	cmdStr, editInTerminal := self.git.File.GetEditCmdStr(filename)
+	return self.callEditor(cmdStr, editInTerminal)
 }
 
 func (self *FilesHelper) EditFileAtLine(filename string, lineNumber int) error {
-	cmdStr, err := self.git.File.GetEditCmdStr(filename, lineNumber)
-	if err != nil {
-		return self.c.Error(err)
+	cmdStr, editInTerminal := self.git.File.GetEditAtLineCmdStr(filename, lineNumber)
+	return self.callEditor(cmdStr, editInTerminal)
+}
+
+func (self *FilesHelper) EditFileAtLineAndWait(filename string, lineNumber int) error {
+	cmdStr := self.git.File.GetEditAtLineAndWaitCmdStr(filename, lineNumber)
+
+	// Always suspend, regardless of the value of the editInTerminal config,
+	// since we want to prevent interacting with the UI until the editor
+	// returns, even if the editor doesn't use the terminal
+	return self.callEditor(cmdStr, true)
+}
+
+func (self *FilesHelper) callEditor(cmdStr string, editInTerminal bool) error {
+	if editInTerminal {
+		return self.c.RunSubprocessAndRefresh(
+			self.os.Cmd.NewShell(cmdStr),
+		)
 	}
 
-	self.c.LogAction(self.c.Tr.Actions.EditFile)
-	return self.c.RunSubprocessAndRefresh(
-		self.os.Cmd.NewShell(cmdStr),
-	)
+	return self.os.Cmd.NewShell(cmdStr).Run()
 }
 
 func (self *FilesHelper) OpenFile(filename string) error {

pkg/gui/controllers/staging_controller.go

@@ -247,7 +247,7 @@ func (self *StagingController) editHunk() error {
 
 	lineOffset := 3
 	lineIdxInHunk := state.GetSelectedLineIdx() - hunkStartIdx
-	if err := self.helpers.Files.EditFileAtLine(patchFilepath, lineIdxInHunk+lineOffset); err != nil {
+	if err := self.helpers.Files.EditFileAtLineAndWait(patchFilepath, lineIdxInHunk+lineOffset); err != nil {
 		return err
 	}