From ae0d88f855773dd76432487509f9af13f987a812 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Fri, 31 Aug 2018 18:43:54 +1000 Subject: [PATCH 01/10] WIP using runDirectCommand with xdg-open --- pkg/app/app.go | 2 +- pkg/commands/os.go | 54 +++++++++-------------------- pkg/commands/os_default_platform.go | 1 + pkg/commands/os_linux.go | 15 ++++++++ pkg/commands/os_windows.go | 1 + pkg/config/app_config.go | 4 +-- pkg/gui/files_panel.go | 27 +++------------ pkg/gui/keybindings.go | 2 -- pkg/gui/status_panel.go | 2 +- pkg/utils/utils.go | 8 +++++ 10 files changed, 51 insertions(+), 65 deletions(-) create mode 100644 pkg/commands/os_linux.go diff --git a/pkg/app/app.go b/pkg/app/app.go index ffd8807f0..fa2415fc3 100644 --- a/pkg/app/app.go +++ b/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) diff --git a/pkg/commands/os.go b/pkg/commands/os.go index 0fa92f724..fd27d39da 100644 --- a/pkg/commands/os.go +++ b/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, @@ -74,12 +78,10 @@ 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. - Command(c.Platform.shell, args...). + Command(c.Platform.shell, c.Platform.shellArg, command). CombinedOutput(), ) } @@ -95,45 +97,23 @@ func sanitisedCommandOutput(output []byte, err error) (string, error) { } // 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": "", +func (c *OSCommand) getOpenCommand() string { + if c.Config.GetUserConfig().IsSet("os.openCommand") { + return c.Config.GetUserConfig().GetString("os.openCommand") } - - 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) + return c.Platform.openCommand } // OpenFile opens a file with the given func (c *OSCommand) OpenFile(filename string) error { - cmdName, cmdTrail, err := c.getOpenCommand() - if err != nil { - return err + commandTemplate := c.getOpenCommand() + templateValues := map[string]string{ + "filename": c.Quote(filename), } - return c.RunCommand(cmdName + " " + c.Quote(filename) + cmdTrail) // TODO: test on linux + command := utils.ResolvePlaceholderString(commandTemplate, templateValues) + _, err := c.RunDirectCommand(command) + return err } // EditFile opens a file in a subprocess using whatever editor is available, diff --git a/pkg/commands/os_default_platform.go b/pkg/commands/os_default_platform.go index f6ac1b515..f106bbd62 100644 --- a/pkg/commands/os_default_platform.go +++ b/pkg/commands/os_default_platform.go @@ -12,5 +12,6 @@ func getPlatform() *Platform { shell: "bash", shellArg: "-c", escapedQuote: "\"", + openCommand: "open {{filename}}", } } diff --git a/pkg/commands/os_linux.go b/pkg/commands/os_linux.go new file mode 100644 index 000000000..cbd037139 --- /dev/null +++ b/pkg/commands/os_linux.go @@ -0,0 +1,15 @@ +package commands + +import ( + "runtime" +) + +func getPlatform() *Platform { + return &Platform{ + os: runtime.GOOS, + shell: "bash", + shellArg: "-c", + escapedQuote: "\"", + openCommand: "xdg-open {{filename}} &>/dev/null &", + } +} diff --git a/pkg/commands/os_windows.go b/pkg/commands/os_windows.go index 28dd7a982..5b50d2c38 100644 --- a/pkg/commands/os_windows.go +++ b/pkg/commands/os_windows.go @@ -6,5 +6,6 @@ func getPlatform() *Platform { shell: "cmd", shellArg: "/c", escapedQuote: "\\\"", + openCommand: "cygstart {{filename}}", } } diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index c85b8e5eb..dce027c8a 100644 --- a/pkg/config/app_config.go +++ b/pkg/config/app_config.go @@ -218,9 +218,9 @@ func GetDefaultConfig() []byte { - white optionsTextColor: - blue -git: +# git: # stuff relating to git -os: +# os: # stuff relating to the OS update: method: prompt # can be: prompt | background | never diff --git a/pkg/gui/files_panel.go b/pkg/gui/files_panel.go index e7fbc5baf..a4c187c9c 100644 --- a/pkg/gui/files_panel.go +++ b/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 { - - sub, err := open(filename) +func (gui *Gui) editFile(filename string) error { + sub, err := gui.OSCommand.EditFile(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) } diff --git a/pkg/gui/keybindings.go b/pkg/gui/keybindings.go index 52496b918..975f5c299 100644 --- a/pkg/gui/keybindings.go +++ b/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}, diff --git a/pkg/gui/status_panel.go b/pkg/gui/status_panel.go index a9dee6a7f..583d7805a 100644 --- a/pkg/gui/status_panel.go +++ b/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 { diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 62706559e..1d5f0f273 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -90,3 +90,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 +} From 3f14b764d5af8ba0390d8f791389330ec1739c9e Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sat, 1 Sep 2018 12:13:41 +1000 Subject: [PATCH 02/10] update tests --- pkg/commands/os.go | 3 +- pkg/commands/os_test.go | 80 ++++++++++++++++++++++------------------- 2 files changed, 44 insertions(+), 39 deletions(-) diff --git a/pkg/commands/os.go b/pkg/commands/os.go index fd27d39da..c3574f98e 100644 --- a/pkg/commands/os.go +++ b/pkg/commands/os.go @@ -80,8 +80,7 @@ func (c *OSCommand) RunDirectCommand(command string) (string, error) { c.Log.WithField("command", command).Info("RunDirectCommand") return sanitisedCommandOutput( - exec. - Command(c.Platform.shell, c.Platform.shellArg, command). + c.command(c.Platform.shell, c.Platform.shellArg, command). CombinedOutput(), ) } diff --git a/pkg/commands/os_test.go b/pkg/commands/os_test.go index a7ccef560..21d72dc67 100644 --- a/pkg/commands/os_test.go +++ b/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) { @@ -60,39 +77,43 @@ func TestOSCommandRunCommand(t *testing.T) { } func TestOSCommandGetOpenCommand(t *testing.T) { + // two scenarios to test. One with a config set, the other with the platform default + type scenario struct { - command func(string, ...string) *exec.Cmd - test func(string, string, error) + before func(*OSCommand) + test func(string) } 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(OSCmd *OSCommand) {}, + func(command string) { + assert.Equal(t, command, "open {{filename}}") }, }, { - 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(OSCmd *OSCommand) { + OSCmd.Config.GetUserConfig().Set("os.openCommand", "test {{filename}}") }, - 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) + func(command string) { + assert.Equal(t, command, "test {{filename}}") + }, + }, + { + func(OSCmd *OSCommand) { + OSCmd.Platform = &Platform{ + openCommand: "platform specific open {{filename}}", + } + }, + func(command string) { + assert.Equal(t, command, "platform specific open {{filename}}") }, }, } for _, s := range scenarios { OSCmd := newDummyOSCommand() - OSCmd.command = s.command - + s.before(OSCmd) s.test(OSCmd.getOpenCommand()) } } @@ -111,29 +132,14 @@ 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" { - return exec.Command("echo") - } - - switch len(arg) { - case 1: - assert.Regexp(t, "open|cygstart", name) - assert.EqualValues(t, "test", arg[0]) - case 3: - assert.Equal(t, "xdg-open", name) - assert.EqualValues(t, "test", arg[0]) - assert.Regexp(t, " \\&\\>/dev/null \\&|", arg[1]) - assert.EqualValues(t, "&", arg[2]) - default: - assert.Fail(t, "Unexisting command given") - } - + assert.Equal(t, name, "bash") + assert.Equal(t, arg, []string{"-c", "open \"test\""}) return exec.Command("echo") }, func(err error) { From f127ae62bb67e86360a1a38971fef7e8e53cb5b9 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sat, 1 Sep 2018 12:15:52 +1000 Subject: [PATCH 03/10] update config --- docs/Config.md | 4 +--- pkg/config/app_config.go | 8 ++++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/docs/Config.md b/docs/Config.md index b2711b603..02b377b95 100644 --- a/docs/Config.md +++ b/docs/Config.md @@ -14,10 +14,8 @@ - white optionsTextColor: - blue - git: - # stuff relating to git os: - # stuff relating to the OS + openCommand: 'open {{filename}}' update: method: prompt # can be: prompt | background | never days: 14 # how often an update is checked for diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index dce027c8a..b5503d6fd 100644 --- a/pkg/config/app_config.go +++ b/pkg/config/app_config.go @@ -218,14 +218,14 @@ 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 reporting: 'undetermined' # one of: 'on' | 'off' | 'undetermined' +# git: +# stuff relating to git +# os: +# openCommand: 'code -r {{filename}}' `) } From 04d5a473d758090b0f65af5520e91bc7924648a1 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sat, 1 Sep 2018 12:22:33 +1000 Subject: [PATCH 04/10] use start instead of cygstart to open files on windows --- pkg/commands/os.go | 2 +- pkg/commands/os_linux.go | 2 +- pkg/commands/os_windows.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/commands/os.go b/pkg/commands/os.go index c3574f98e..680105df8 100644 --- a/pkg/commands/os.go +++ b/pkg/commands/os.go @@ -111,7 +111,7 @@ func (c *OSCommand) OpenFile(filename string) error { } command := utils.ResolvePlaceholderString(commandTemplate, templateValues) - _, err := c.RunDirectCommand(command) + err := c.RunCommand(command) return err } diff --git a/pkg/commands/os_linux.go b/pkg/commands/os_linux.go index cbd037139..c17ebc98f 100644 --- a/pkg/commands/os_linux.go +++ b/pkg/commands/os_linux.go @@ -10,6 +10,6 @@ func getPlatform() *Platform { shell: "bash", shellArg: "-c", escapedQuote: "\"", - openCommand: "xdg-open {{filename}} &>/dev/null &", + openCommand: "bash -c \"xdg-open {{filename}} &>/dev/null &\"", } } diff --git a/pkg/commands/os_windows.go b/pkg/commands/os_windows.go index 5b50d2c38..f41c6d2b9 100644 --- a/pkg/commands/os_windows.go +++ b/pkg/commands/os_windows.go @@ -6,6 +6,6 @@ func getPlatform() *Platform { shell: "cmd", shellArg: "/c", escapedQuote: "\\\"", - openCommand: "cygstart {{filename}}", + openCommand: "start {{filename}}", } } From 865809e625330d672f428630d1faae2968b6b7b1 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sat, 1 Sep 2018 13:27:58 +1000 Subject: [PATCH 05/10] better error handling for commands --- pkg/commands/os.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/commands/os.go b/pkg/commands/os.go index 680105df8..9fc3a2bc0 100644 --- a/pkg/commands/os.go +++ b/pkg/commands/os.go @@ -90,6 +90,9 @@ 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 @@ -111,7 +114,7 @@ func (c *OSCommand) OpenFile(filename string) error { } command := utils.ResolvePlaceholderString(commandTemplate, templateValues) - err := c.RunCommand(command) + _, err := c.RunCommandWithOutput(command) return err } From ad880e2d56cd9ab3841dd077d47979202c509460 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sat, 1 Sep 2018 13:28:10 +1000 Subject: [PATCH 06/10] wrap windows start command in shell --- pkg/commands/os_windows.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/commands/os_windows.go b/pkg/commands/os_windows.go index f41c6d2b9..51ad401cd 100644 --- a/pkg/commands/os_windows.go +++ b/pkg/commands/os_windows.go @@ -5,7 +5,6 @@ func getPlatform() *Platform { os: "windows", shell: "cmd", shellArg: "/c", - escapedQuote: "\\\"", - openCommand: "start {{filename}}", - } -} + escapedQuote: `\"`, + openCommand: `cmd /c "start "" {{filename}}"`, +}} From d31520261f0ffc4cb93e29476e39ace45e813fc8 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sat, 1 Sep 2018 14:33:01 +1000 Subject: [PATCH 07/10] introduce platform specific defaults --- pkg/commands/os.go | 13 +----- pkg/commands/os_linux.go | 15 ------- pkg/commands/os_test.go | 58 +++++++-------------------- pkg/commands/os_windows.go | 4 +- pkg/config/app_config.go | 20 +++++---- pkg/config/config_default_platform.go | 10 +++++ pkg/config/config_linux.go | 8 ++++ pkg/config/config_windows.go | 8 ++++ 8 files changed, 53 insertions(+), 83 deletions(-) delete mode 100644 pkg/commands/os_linux.go create mode 100644 pkg/config/config_default_platform.go create mode 100644 pkg/config/config_linux.go create mode 100644 pkg/config/config_windows.go diff --git a/pkg/commands/os.go b/pkg/commands/os.go index 9fc3a2bc0..834c45376 100644 --- a/pkg/commands/os.go +++ b/pkg/commands/os.go @@ -51,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(), ) @@ -98,23 +97,15 @@ func sanitisedCommandOutput(output []byte, err error) (string, error) { return outputString, nil } -// getOpenCommand get open command -func (c *OSCommand) getOpenCommand() string { - if c.Config.GetUserConfig().IsSet("os.openCommand") { - return c.Config.GetUserConfig().GetString("os.openCommand") - } - return c.Platform.openCommand -} - // OpenFile opens a file with the given func (c *OSCommand) OpenFile(filename string) error { - commandTemplate := c.getOpenCommand() + commandTemplate := c.Config.GetUserConfig().GetString("os.openCommand") templateValues := map[string]string{ "filename": c.Quote(filename), } command := utils.ResolvePlaceholderString(commandTemplate, templateValues) - _, err := c.RunCommandWithOutput(command) + err := c.RunCommand(command) return err } diff --git a/pkg/commands/os_linux.go b/pkg/commands/os_linux.go deleted file mode 100644 index c17ebc98f..000000000 --- a/pkg/commands/os_linux.go +++ /dev/null @@ -1,15 +0,0 @@ -package commands - -import ( - "runtime" -) - -func getPlatform() *Platform { - return &Platform{ - os: runtime.GOOS, - shell: "bash", - shellArg: "-c", - escapedQuote: "\"", - openCommand: "bash -c \"xdg-open {{filename}} &>/dev/null &\"", - } -} diff --git a/pkg/commands/os_test.go b/pkg/commands/os_test.go index 21d72dc67..1eab1580d 100644 --- a/pkg/commands/os_test.go +++ b/pkg/commands/os_test.go @@ -76,48 +76,6 @@ func TestOSCommandRunCommand(t *testing.T) { } } -func TestOSCommandGetOpenCommand(t *testing.T) { - // two scenarios to test. One with a config set, the other with the platform default - - type scenario struct { - before func(*OSCommand) - test func(string) - } - - scenarios := []scenario{ - { - func(OSCmd *OSCommand) {}, - func(command string) { - assert.Equal(t, command, "open {{filename}}") - }, - }, - { - func(OSCmd *OSCommand) { - OSCmd.Config.GetUserConfig().Set("os.openCommand", "test {{filename}}") - }, - func(command string) { - assert.Equal(t, command, "test {{filename}}") - }, - }, - { - func(OSCmd *OSCommand) { - OSCmd.Platform = &Platform{ - openCommand: "platform specific open {{filename}}", - } - }, - func(command string) { - assert.Equal(t, command, "platform specific open {{filename}}") - }, - }, - } - - for _, s := range scenarios { - OSCmd := newDummyOSCommand() - s.before(OSCmd) - s.test(OSCmd.getOpenCommand()) - } -} - func TestOSCommandOpenFile(t *testing.T) { type scenario struct { filename string @@ -138,8 +96,19 @@ func TestOSCommandOpenFile(t *testing.T) { { "test", func(name string, arg ...string) *exec.Cmd { - assert.Equal(t, name, "bash") - assert.Equal(t, arg, []string{"-c", "open \"test\""}) + assert.Equal(t, "open", name) + assert.Equal(t, []string{"test"}, arg) + return exec.Command("echo") + }, + func(err error) { + assert.NoError(t, err) + }, + }, + { + "filename with spaces", + func(name string, arg ...string) *exec.Cmd { + assert.Equal(t, "open", name) + assert.Equal(t, []string{"filename with spaces"}, arg) return exec.Command("echo") }, func(err error) { @@ -151,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)) } diff --git a/pkg/commands/os_windows.go b/pkg/commands/os_windows.go index 51ad401cd..1658e5f36 100644 --- a/pkg/commands/os_windows.go +++ b/pkg/commands/os_windows.go @@ -6,5 +6,5 @@ func getPlatform() *Platform { shell: "cmd", shellArg: "/c", escapedQuote: `\"`, - openCommand: `cmd /c "start "" {{filename}}"`, -}} + } +} diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index b5503d6fd..3e384d80f 100644 --- a/pkg/config/app_config.go +++ b/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", defaultConfig) + userConfig, err := LoadConfig("config", true) 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 err = LoadDefaults(v, defaults); err != nil { + if withDefaults { + 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 } @@ -222,10 +224,6 @@ update: method: prompt # can be: prompt | background | never days: 14 # how often a update is checked for reporting: 'undetermined' # one of: 'on' | 'off' | 'undetermined' -# git: -# stuff relating to git -# os: -# openCommand: 'code -r {{filename}}' `) } diff --git a/pkg/config/config_default_platform.go b/pkg/config/config_default_platform.go new file mode 100644 index 000000000..a87d5a7f6 --- /dev/null +++ b/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}}'`) +} diff --git a/pkg/config/config_linux.go b/pkg/config/config_linux.go new file mode 100644 index 000000000..ef30ac7d2 --- /dev/null +++ b/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 &\"'`) +} diff --git a/pkg/config/config_windows.go b/pkg/config/config_windows.go new file mode 100644 index 000000000..b81a5fdb5 --- /dev/null +++ b/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}}"'`) +} From 3cafa2bb12578c89137b43c159b900878c4d193a Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sat, 1 Sep 2018 14:35:46 +1000 Subject: [PATCH 08/10] update config to reflect platform specific defaults --- docs/Config.md | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/docs/Config.md b/docs/Config.md index 02b377b95..d428603bc 100644 --- a/docs/Config.md +++ b/docs/Config.md @@ -14,14 +14,44 @@ - white optionsTextColor: - blue - os: - openCommand: 'open {{filename}}' 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) From 87de803a6c34bff20330645a220381a1172d0b3a Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sat, 1 Sep 2018 14:38:30 +1000 Subject: [PATCH 09/10] update default config header --- pkg/config/config_default_platform.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/config_default_platform.go b/pkg/config/config_default_platform.go index a87d5a7f6..f3c1a36e5 100644 --- a/pkg/config/config_default_platform.go +++ b/pkg/config/config_default_platform.go @@ -1,4 +1,4 @@ -// +build !windows !linux +// +build !windows,!linux package config From a9cd27707015a99df34aefd27471328ca23c8b3f Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Mon, 3 Sep 2018 19:31:27 +1000 Subject: [PATCH 10/10] add test for ResolvePlaceholderString --- pkg/utils/utils_test.go | 53 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index 46b264945..0b2d35959 100644 --- a/pkg/utils/utils_test.go +++ b/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)) + } +}