git: fix edit command when using extra args

The user can include extra arguments to the git core.editor configuration
option or the GIT_EDITOR env var, however, in the current way that
editorCMD() is coded, all the extra arguments are passed to the actual
editor executable as a single argument: argv[0] == exec, argv[1] == all
arguments, instead of splitting them in separated argv[] fields. Although
some editors are fine with this behavior, because they split the argv array
before actually using it, some others don't.

Since splitting all the args, one per argv field, is the standard across the
industry, we should do the same. This patch refactors the editorCMD() code
to ease its understanding, split each arg in one argv field, and also to
solve an issue, found during the refactoring, for when the user hasn't set
neither git core.editor nor GIT_EDITOR.

Signed-off-by: Bruno Meneguele <bmeneg@redhat.com>

internal/git/edit.go

@@ -37,7 +37,7 @@ func EditFile(filePrefix, message string) (string, error) {
 		dir = "/tmp"
 	}
 	filePath := filepath.Join(dir, fmt.Sprintf("%s_EDITMSG", filePrefix))
-	editorPath, err := editorPath()
+	editor, err := editor()
 	if err != nil {
 		return "", err
 	}
@@ -51,7 +51,7 @@ func EditFile(filePrefix, message string) (string, error) {
 		}
 	}
 
-	cmd := editorCMD(editorPath, filePath)
+	cmd := editorCMD(editor, filePath)
 	err = cmd.Run()
 	if err != nil {
 		fmt.Printf("ERROR(cmd): Saved file contents written to %s\n", filePath)
@@ -68,7 +68,7 @@ func EditFile(filePrefix, message string) (string, error) {
 	return removeComments(string(contents))
 }
 
-func editorPath() (string, error) {
+func editor() (string, error) {
 	cmd := New("var", "GIT_EDITOR")
 	cmd.Stdout = nil
 	a, err := cmd.Output()
@@ -88,21 +88,29 @@ func editorPath() (string, error) {
 	return editor, nil
 }
 
-func editorCMD(editorPath, filePath string) *exec.Cmd {
-	parts := strings.Split(editorPath, " ")
+func editorCMD(editor, filePath string) *exec.Cmd {
+	// make 'vi' the default editor to avoid empty editor configs
+	if editor == "" {
+		editor = "vi"
+	}
+
 	r := regexp.MustCompile("[nmg]?vi[m]?$")
 	args := make([]string, 0, 3)
-	if r.MatchString(editorPath) {
+	if r.MatchString(editor) {
 		args = append(args, "--cmd", "set ft=gitcommit tw=0 wrap lbr")
 	}
-	argparts := strings.Join(parts[1:], " ")
-	if len(argparts) == 0 {
-		args = append(args, filePath)
-	} else {
-		argparts = strings.Replace(argparts, "'", "\"", -1)
-		args = append(args, argparts, filePath)
+
+	parts := strings.Split(editor, " ")
+	name := parts[0]
+	if len(parts) > 0 {
+		for _, arg := range parts[1:] {
+			arg = strings.Replace(arg, "'", "\"", -1)
+			args = append(args, arg)
+		}
 	}
-	cmd := exec.Command(parts[0], args...)
+	args = append(args, filePath)
+
+	cmd := exec.Command(name, args...)
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr

internal/git/edit_test.go

@@ -81,14 +81,14 @@ func TestEditor(t *testing.T) {
 	}
 
 	var path string
-	t.Run("editorPath()", func(t *testing.T) {
+	t.Run("editor()", func(t *testing.T) {
 		var err error
-		path, err = editorPath()
+		path, err = editor()
 		if err != nil {
 			t.Fatal(err)
 		}
 
-		require.NotEmpty(t, editorPath)
+		require.NotEmpty(t, editor)
 	})
 	t.Run("Open Editor", func(t *testing.T) {
 		cmd := editorCMD(path, filePath)