Merge pull request #249 from jesseduffield/hotfix/238-xdg-open

238: opening with xdg-open

docs/Config.md

@@ -14,16 +14,44 @@
         - white
       optionsTextColor:
         - blue
-  git:
-    # stuff relating to git
-  os:
-    # stuff relating to the OS
   update:
     method: prompt # can be: prompt | background | never
     days: 14 # how often an update is checked for
   reporting: 'undetermined' # one of: 'on' | 'off' | 'undetermined'
 ```
 
+## Platform Defaults:
+
+### Windows:
+
+```
+  os:
+    openCommand: 'cmd /c "start "" {{filename}}"'
+```
+
+### Linux:
+
+```
+  os:
+    openCommand: 'bash -c \"xdg-open {{filename}} &>/dev/null &\"'
+```
+
+### OSX:
+
+```
+  os:
+    openCommand: 'open {{filename}}'
+```
+
+### Recommended Config Values:
+
+for users of VSCode
+
+```
+  os:
+    openCommand: 'code -r {{filename}}'
+```
+
 ## Color Attributes:
 
 For color attributes you can choose an array of attributes (with max one color attribute)

pkg/app/app.go

@@ -73,7 +73,7 @@ func NewApp(config config.AppConfigurer) (*App, error) {
 	}
 	var err error
 	app.Log = newLogger(config)
-	app.OSCommand = commands.NewOSCommand(app.Log)
+	app.OSCommand = commands.NewOSCommand(app.Log, config)
 
 	app.Tr = i18n.NewLocalizer(app.Log)
 

pkg/commands/os.go

@@ -6,7 +6,8 @@ import (
 	"os/exec"
 	"strings"
 
-	"github.com/davecgh/go-spew/spew"
+	"github.com/jesseduffield/lazygit/pkg/config"
+	"github.com/jesseduffield/lazygit/pkg/utils"
 
 	"github.com/mgutz/str"
 
@@ -20,22 +21,25 @@ type Platform struct {
 	shell        string
 	shellArg     string
 	escapedQuote string
+	openCommand  string
 }
 
 // OSCommand holds all the os commands
 type OSCommand struct {
 	Log                *logrus.Entry
 	Platform           *Platform
+	Config             config.AppConfigurer
 	command            func(string, ...string) *exec.Cmd
 	getGlobalGitConfig func(string) (string, error)
 	getenv             func(string) string
 }
 
 // NewOSCommand os command runner
-func NewOSCommand(log *logrus.Entry) *OSCommand {
+func NewOSCommand(log *logrus.Entry, config config.AppConfigurer) *OSCommand {
 	return &OSCommand{
 		Log:                log,
 		Platform:           getPlatform(),
+		Config:             config,
 		command:            exec.Command,
 		getGlobalGitConfig: gitconfig.Global,
 		getenv:             os.Getenv,
@@ -47,7 +51,6 @@ func (c *OSCommand) RunCommandWithOutput(command string) (string, error) {
 	c.Log.WithField("command", command).Info("RunCommand")
 	splitCmd := str.ToArgv(command)
 	c.Log.Info(splitCmd)
-
 	return sanitisedCommandOutput(
 		c.command(splitCmd[0], splitCmd[1:]...).CombinedOutput(),
 	)
@@ -74,12 +77,9 @@ func (c *OSCommand) FileType(path string) string {
 // RunDirectCommand wrapper around direct commands
 func (c *OSCommand) RunDirectCommand(command string) (string, error) {
 	c.Log.WithField("command", command).Info("RunDirectCommand")
-	args := str.ToArgv(c.Platform.shellArg + " " + command)
-	c.Log.Info(spew.Sdump(args))
 
 	return sanitisedCommandOutput(
-		exec.
+		c.command(c.Platform.shell, c.Platform.shellArg, command).
-			Command(c.Platform.shell, args...).
 			CombinedOutput(),
 	)
 }
@@ -89,51 +89,24 @@ func sanitisedCommandOutput(output []byte, err error) (string, error) {
 	if err != nil {
 		// errors like 'exit status 1' are not very useful so we'll create an error
 		// from the combined output
+		if outputString == "" {
+			return "", err
+		}
 		return outputString, errors.New(outputString)
 	}
 	return outputString, nil
 }
 
-// getOpenCommand get open command
-func (c *OSCommand) getOpenCommand() (string, string, error) {
-	//NextStep open equivalents: xdg-open (linux), cygstart (cygwin), open (OSX)
-	trailMap := map[string]string{
-		"xdg-open": " &>/dev/null &",
-		"cygstart": "",
-		"open":     "",
-	}
-
-	for name, trail := range trailMap {
-		if err := c.RunCommand("which " + name); err == nil {
-			return name, trail, nil
-		}
-	}
-	return "", "", errors.New("Unsure what command to use to open this file")
-}
-
-// VsCodeOpenFile opens the file in code, with the -r flag to open in the
-// current window
-// each of these open files needs to have the same function signature because
-// they're being passed as arguments into another function,
-// but only editFile actually returns a *exec.Cmd
-func (c *OSCommand) VsCodeOpenFile(filename string) (*exec.Cmd, error) {
-	return nil, c.RunCommand("code -r " + filename)
-}
-
-// SublimeOpenFile opens the filein sublime
-// may be deprecated in the future
-func (c *OSCommand) SublimeOpenFile(filename string) (*exec.Cmd, error) {
-	return nil, c.RunCommand("subl " + filename)
-}
-
 // OpenFile opens a file with the given
 func (c *OSCommand) OpenFile(filename string) error {
-	cmdName, cmdTrail, err := c.getOpenCommand()
+	commandTemplate := c.Config.GetUserConfig().GetString("os.openCommand")
-	if err != nil {
+	templateValues := map[string]string{
-		return err
+		"filename": c.Quote(filename),
 	}
 
-	return c.RunCommand(cmdName + " " + c.Quote(filename) + cmdTrail) // TODO: test on linux
+	command := utils.ResolvePlaceholderString(commandTemplate, templateValues)
+	err := c.RunCommand(command)
+	return err
 }
 
 // EditFile opens a file in a subprocess using whatever editor is available,

pkg/commands/os_default_platform.go

@@ -12,5 +12,6 @@ func getPlatform() *Platform {
 		shell:        "bash",
 		shellArg:     "-c",
 		escapedQuote: "\"",
+		openCommand:  "open {{filename}}",
 	}
 }

pkg/commands/os_test.go

@@ -5,11 +5,28 @@ import (
 	"os/exec"
 	"testing"
 
+	"github.com/jesseduffield/lazygit/pkg/config"
+	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
+	yaml "gopkg.in/yaml.v2"
 )
 
 func newDummyOSCommand() *OSCommand {
-	return NewOSCommand(newDummyLog())
+	return NewOSCommand(newDummyLog(), newDummyAppConfig())
+}
+
+func newDummyAppConfig() *config.AppConfig {
+	appConfig := &config.AppConfig{
+		Name:        "lazygit",
+		Version:     "unversioned",
+		Commit:      "",
+		BuildDate:   "",
+		Debug:       false,
+		BuildSource: "",
+		UserConfig:  viper.New(),
+	}
+	_ = yaml.Unmarshal([]byte{}, appConfig.AppState)
+	return appConfig
 }
 
 func TestOSCommandRunCommandWithOutput(t *testing.T) {
@@ -59,44 +76,6 @@ func TestOSCommandRunCommand(t *testing.T) {
 	}
 }
 
-func TestOSCommandGetOpenCommand(t *testing.T) {
-	type scenario struct {
-		command func(string, ...string) *exec.Cmd
-		test    func(string, string, error)
-	}
-
-	scenarios := []scenario{
-		{
-			func(name string, arg ...string) *exec.Cmd {
-				return exec.Command("exit", "1")
-			},
-			func(name string, trail string, err error) {
-				assert.EqualError(t, err, "Unsure what command to use to open this file")
-			},
-		},
-		{
-			func(name string, arg ...string) *exec.Cmd {
-				assert.Equal(t, "which", name)
-				assert.Len(t, arg, 1)
-				assert.Regexp(t, "xdg-open|cygstart|open", arg[0])
-				return exec.Command("echo")
-			},
-			func(name string, trail string, err error) {
-				assert.NoError(t, err)
-				assert.Regexp(t, "xdg-open|cygstart|open", name)
-				assert.Regexp(t, " \\&\\>/dev/null \\&|", trail)
-			},
-		},
-	}
-
-	for _, s := range scenarios {
-		OSCmd := newDummyOSCommand()
-		OSCmd.command = s.command
-
-		s.test(OSCmd.getOpenCommand())
-	}
-}
-
 func TestOSCommandOpenFile(t *testing.T) {
 	type scenario struct {
 		filename string
@@ -111,29 +90,25 @@ func TestOSCommandOpenFile(t *testing.T) {
 				return exec.Command("exit", "1")
 			},
 			func(err error) {
-				assert.EqualError(t, err, "Unsure what command to use to open this file")
+				assert.Error(t, err)
 			},
 		},
 		{
 			"test",
 			func(name string, arg ...string) *exec.Cmd {
-				if name == "which" {
+				assert.Equal(t, "open", name)
-					return exec.Command("echo")
+				assert.Equal(t, []string{"test"}, arg)
-				}
+				return exec.Command("echo")
-
+			},
-				switch len(arg) {
+			func(err error) {
-				case 1:
+				assert.NoError(t, err)
-					assert.Regexp(t, "open|cygstart", name)
+			},
-					assert.EqualValues(t, "test", arg[0])
+		},
-				case 3:
+		{
-					assert.Equal(t, "xdg-open", name)
+			"filename with spaces",
-					assert.EqualValues(t, "test", arg[0])
+			func(name string, arg ...string) *exec.Cmd {
-					assert.Regexp(t, " \\&\\>/dev/null \\&|", arg[1])
+				assert.Equal(t, "open", name)
-					assert.EqualValues(t, "&", arg[2])
+				assert.Equal(t, []string{"filename with spaces"}, arg)
-				default:
-					assert.Fail(t, "Unexisting command given")
-				}
-
 				return exec.Command("echo")
 			},
 			func(err error) {
@@ -145,6 +120,7 @@ func TestOSCommandOpenFile(t *testing.T) {
 	for _, s := range scenarios {
 		OSCmd := newDummyOSCommand()
 		OSCmd.command = s.command
+		OSCmd.Config.GetUserConfig().Set("os.openCommand", "open {{filename}}")
 
 		s.test(OSCmd.OpenFile(s.filename))
 	}

pkg/commands/os_windows.go

@@ -5,6 +5,6 @@ func getPlatform() *Platform {
 		os:           "windows",
 		shell:        "cmd",
 		shellArg:     "/c",
-		escapedQuote: "\\\"",
+		escapedQuote: `\"`,
 	}
 }

pkg/config/app_config.go

@@ -40,8 +40,7 @@ type AppConfigurer interface {
 
 // NewAppConfig makes a new app config
 func NewAppConfig(name, version, commit, date string, buildSource string, debuggingFlag *bool) (*AppConfig, error) {
-	defaultConfig := GetDefaultConfig()
+	userConfig, err := LoadConfig("config", true)
-	userConfig, err := LoadConfig("config", defaultConfig)
 	if err != nil {
 		return nil, err
 	}
@@ -113,13 +112,16 @@ func newViper(filename string) (*viper.Viper, error) {
 }
 
 // LoadConfig gets the user's config
-func LoadConfig(filename string, defaults []byte) (*viper.Viper, error) {
+func LoadConfig(filename string, withDefaults bool) (*viper.Viper, error) {
 	v, err := newViper(filename)
 	if err != nil {
 		return nil, err
 	}
-	if defaults != nil {
+	if withDefaults {
-		if err = LoadDefaults(v, defaults); err != nil {
+		if err = LoadDefaults(v, GetDefaultConfig()); err != nil {
+			return nil, err
+		}
+		if err = LoadDefaults(v, GetPlatformDefaultConfig()); err != nil {
 			return nil, err
 		}
 	}
@@ -131,7 +133,7 @@ func LoadConfig(filename string, defaults []byte) (*viper.Viper, error) {
 
 // LoadDefaults loads in the defaults defined in this file
 func LoadDefaults(v *viper.Viper, defaults []byte) error {
-	return v.ReadConfig(bytes.NewBuffer(defaults))
+	return v.MergeConfig(bytes.NewBuffer(defaults))
 }
 
 func prepareConfigFile(filename string) (string, error) {
@@ -166,7 +168,7 @@ func LoadAndMergeFile(v *viper.Viper, filename string) error {
 func (c *AppConfig) WriteToUserConfig(key, value string) error {
 	// reloading the user config directly (without defaults) so that we're not
 	// writing any defaults back to the user's config
-	v, err := LoadConfig("config", nil)
+	v, err := LoadConfig("config", false)
 	if err != nil {
 		return err
 	}
@@ -220,10 +222,6 @@ func GetDefaultConfig() []byte {
       - white
     optionsTextColor:
       - blue
-git:
-  # stuff relating to git
-os:
-  # stuff relating to the OS
 update:
   method: prompt # can be: prompt | background | never
   days: 14 # how often a update is checked for

pkg/config/config_default_platform.go

@@ -0,0 +1,10 @@
+// +build !windows,!linux
+
+package config
+
+// GetPlatformDefaultConfig gets the defaults for the platform
+func GetPlatformDefaultConfig() []byte {
+	return []byte(
+		`os:
+  openCommand: 'open {{filename}}'`)
+}

pkg/config/config_linux.go

@@ -0,0 +1,8 @@
+package config
+
+// GetPlatformDefaultConfig gets the defaults for the platform
+func GetPlatformDefaultConfig() []byte {
+	return []byte(
+		`os:
+  openCommand: 'bash -c \"xdg-open {{filename}} &>/dev/null &\"'`)
+}

pkg/config/config_windows.go

@@ -0,0 +1,8 @@
+package config
+
+// GetPlatformDefaultConfig gets the defaults for the platform
+func GetPlatformDefaultConfig() []byte {
+	return []byte(
+		`os:
+  openCommand: 'cmd /c "start "" {{filename}}"'`)
+}

pkg/gui/files_panel.go

@@ -7,7 +7,6 @@ import (
 
 	// "strings"
 
-	"os/exec"
 	"strings"
 
 	"github.com/fatih/color"
@@ -250,11 +249,10 @@ func (gui *Gui) PrepareSubProcess(g *gocui.Gui, commands ...string) {
 	})
 }
 
-func (gui *Gui) genericFileOpen(g *gocui.Gui, v *gocui.View, filename string, open func(string) (*exec.Cmd, error)) error {
+func (gui *Gui) editFile(filename string) error {
-
+	sub, err := gui.OSCommand.EditFile(filename)
-	sub, err := open(filename)
 	if err != nil {
-		return gui.createErrorPanel(g, err.Error())
+		return gui.createErrorPanel(gui.g, err.Error())
 	}
 	if sub != nil {
 		gui.SubProcess = sub
@@ -268,7 +266,8 @@ func (gui *Gui) handleFileEdit(g *gocui.Gui, v *gocui.View) error {
 	if err != nil {
 		return err
 	}
-	return gui.genericFileOpen(g, v, file.Name, gui.OSCommand.EditFile)
+
+	return gui.editFile(file.Name)
 }
 
 func (gui *Gui) handleFileOpen(g *gocui.Gui, v *gocui.View) error {
@@ -279,22 +278,6 @@ func (gui *Gui) handleFileOpen(g *gocui.Gui, v *gocui.View) error {
 	return gui.openFile(file.Name)
 }
 
-func (gui *Gui) handleSublimeFileOpen(g *gocui.Gui, v *gocui.View) error {
-	file, err := gui.getSelectedFile(g)
-	if err != nil {
-		return err
-	}
-	return gui.genericFileOpen(g, v, file.Name, gui.OSCommand.SublimeOpenFile)
-}
-
-func (gui *Gui) handleVsCodeFileOpen(g *gocui.Gui, v *gocui.View) error {
-	file, err := gui.getSelectedFile(g)
-	if err != nil {
-		return err
-	}
-	return gui.genericFileOpen(g, v, file.Name, gui.OSCommand.VsCodeOpenFile)
-}
-
 func (gui *Gui) handleRefreshFiles(g *gocui.Gui, v *gocui.View) error {
 	return gui.refreshFiles(g)
 }

pkg/gui/keybindings.go

@@ -34,8 +34,6 @@ func (gui *Gui) keybindings(g *gocui.Gui) error {
 		{ViewName: "files", Key: 'm', Modifier: gocui.ModNone, Handler: gui.handleSwitchToMerge},
 		{ViewName: "files", Key: 'e', Modifier: gocui.ModNone, Handler: gui.handleFileEdit},
 		{ViewName: "files", Key: 'o', Modifier: gocui.ModNone, Handler: gui.handleFileOpen},
-		{ViewName: "files", Key: 's', Modifier: gocui.ModNone, Handler: gui.handleSublimeFileOpen},
-		{ViewName: "files", Key: 'v', Modifier: gocui.ModNone, Handler: gui.handleVsCodeFileOpen},
 		{ViewName: "files", Key: 'i', Modifier: gocui.ModNone, Handler: gui.handleIgnoreFile},
 		{ViewName: "files", Key: 'r', Modifier: gocui.ModNone, Handler: gui.handleRefreshFiles},
 		{ViewName: "files", Key: 'S', Modifier: gocui.ModNone, Handler: gui.handleStashSave},

pkg/gui/status_panel.go

@@ -76,7 +76,7 @@ func (gui *Gui) handleOpenConfig(g *gocui.Gui, v *gocui.View) error {
 
 func (gui *Gui) handleEditConfig(g *gocui.Gui, v *gocui.View) error {
 	filename := gui.Config.GetUserConfig().ConfigFileUsed()
-	return gui.genericFileOpen(g, v, filename, gui.OSCommand.EditFile)
+	return gui.editFile(filename)
 }
 
 func lazygitTitle() string {

pkg/utils/utils.go

@@ -91,3 +91,11 @@ func Loader() string {
 	index := nanos / 50000000 % int64(len(characters))
 	return characters[index : index+1]
 }
+
+// ResolvePlaceholderString populates a template with values
+func ResolvePlaceholderString(str string, arguments map[string]string) string {
+	for key, value := range arguments {
+		str = strings.Replace(str, "{{"+key+"}}", value, -1)
+	}
+	return str
+}

pkg/utils/utils_test.go

@@ -114,3 +114,56 @@ func TestNormalizeLinefeeds(t *testing.T) {
 		assert.EqualValues(t, string(s.expected), NormalizeLinefeeds(string(s.byteArray)))
 	}
 }
+
+func TestResolvePlaceholderString(t *testing.T) {
+	type scenario struct {
+		templateString string
+		arguments      map[string]string
+		expected       string
+	}
+
+	scenarios := []scenario{
+		{
+			"",
+			map[string]string{},
+			"",
+		},
+		{
+			"hello",
+			map[string]string{},
+			"hello",
+		},
+		{
+			"hello {{arg}}",
+			map[string]string{},
+			"hello {{arg}}",
+		},
+		{
+			"hello {{arg}}",
+			map[string]string{"arg": "there"},
+			"hello there",
+		},
+		{
+			"hello",
+			map[string]string{"arg": "there"},
+			"hello",
+		},
+		{
+			"{{nothing}}",
+			map[string]string{"nothing": ""},
+			"",
+		},
+		{
+			"{{}} {{ this }} { should not throw}} an {{{{}}}} error",
+			map[string]string{
+				"blah": "blah",
+				"this": "won't match",
+			},
+			"{{}} {{ this }} { should not throw}} an {{{{}}}} error",
+		},
+	}
+
+	for _, s := range scenarios {
+		assert.EqualValues(t, string(s.expected), ResolvePlaceholderString(s.templateString, s.arguments))
+	}
+}