Fix klog edit if $EDITOR contains spaces; improve error handling

* Fix command splitting to parse quoted args * Print error when tryCommand fails * Separate command and arguments * Fail directly, if $EDITOR was set but can’t be opened Co-authored-by: Jan Heuermann <[email protected]>
jotaen · Aug 17, 2022 · 16c7c63 · 16c7c63
1 parent 070fab4
commit 16c7c63
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 16 deletions.
diff --git a/src/app/context.go b/src/app/context.go
@@ -319,11 +319,13 @@ func (ctx *context) bookmarkDatabasePath() File {
 	return NewFileOrPanic(ctx.KlogFolder() + "bookmarks.json")
 }
 
-func tryCommands(commands []string, additionalArg string) bool {
+// tryCommands tries to execute the given commands one after the other.
+// Returns `true` upon first successful execution; `false` otherwise.
+func tryCommands(commands [][]string, pathArg string) bool {
 	for _, command := range commands {
-		args := strings.Split(command, " ")
-		args = append(args, additionalArg)
-		cmd := exec.Command(args[0], args[1:]...)
+		bin := command[0]
+		binArgs := command[1:]
+		cmd := exec.Command(bin, append(binArgs, pathArg)...)
 		cmd.Stdin = os.Stdin
 		cmd.Stdout = os.Stdout
 		err := cmd.Run()
@@ -354,14 +356,27 @@ func (ctx *context) OpenInEditor(fileArg FileOrBookmarkName, printHint func(stri
 	if err != nil {
 		return err
 	}
-	hint := "You can specify your preferred editor via the $EDITOR environment variable.\n"
+
+	// If $EDITOR is specified, try to open it.
 	preferredEditor := os.Getenv("EDITOR")
-	hasSucceeded := tryCommands(append([]string{preferredEditor}, POTENTIAL_EDITORS...), target.Path())
-	if hasSucceeded {
-		if preferredEditor == "" {
-			// Inform the user that they can configure their editor:
-			printHint(hint)
+	if preferredEditor != "" {
+		hasSucceeded := tryCommands([][]string{{preferredEditor}}, target.Path())
+		if hasSucceeded {
+			return nil
 		}
+		return NewError(
+			"Cannot open preferred editor",
+			"$EDITOR variable was: "+preferredEditor,
+			nil,
+		)
+	}
+
+	// If no $EDITOR is specified, fall back to trying the pre-configured options.
+	hasSucceeded := tryCommands(POTENTIAL_EDITORS, target.Path())
+	hint := "You can specify your preferred editor via the $EDITOR environment variable."
+	if hasSucceeded {
+		// Inform the user that they can configure their editor:
+		printHint(hint)
 		return nil
 	}
 	return NewError(

diff --git a/src/app/sys_darwin.go b/src/app/sys_darwin.go
@@ -2,9 +2,9 @@
 
 package app
 
-var POTENTIAL_EDITORS = []string{"vim", "vi", "nano", "pico", "open -a TextEdit"}
+var POTENTIAL_EDITORS = [][]string{{"vim"}, {"vi"}, {"nano"}, {"pico"}, {"open", "-a", "TextEdit"}}
 
-var POTENTIAL_FILE_EXLORERS = []string{"open"}
+var POTENTIAL_FILE_EXLORERS = [][]string{{"open"}}
 
 func flagAsHidden(path string) {
 	// Nothing to do on UNIX due to the dotfile convention

diff --git a/src/app/sys_linux.go b/src/app/sys_linux.go
@@ -2,9 +2,9 @@
 
 package app
 
-var POTENTIAL_EDITORS = []string{"vim", "vi", "nano", "pico"}
+var POTENTIAL_EDITORS = [][]string{{"vim"}, {"vi"}, {"nano"}, {"pico"}}
 
-var POTENTIAL_FILE_EXLORERS = []string{"xdg-open"}
+var POTENTIAL_FILE_EXLORERS = [][]string{{"xdg-open"}}
 
 func flagAsHidden(path string) {
 	// Nothing to do on UNIX due to the dotfile convention

diff --git a/src/app/sys_windows.go b/src/app/sys_windows.go
@@ -8,9 +8,9 @@ import (
 	"unsafe"
 )
 
-var POTENTIAL_EDITORS = []string{"notepad"}
+var POTENTIAL_EDITORS = [][]string{{"notepad"}}
 
-var POTENTIAL_FILE_EXLORERS = []string{"cmd.exe /C start"}
+var POTENTIAL_FILE_EXLORERS = [][]string{{"cmd.exe", "/C", "start"}}
 
 func flagAsHidden(path string) {
 	winFileName, err := syscall.UTF16PtrFromString(path)