fixed review comments

Author: sathiraumesh

cmd/cli/readline/editor.go

@@ -1,13 +1,76 @@
 package readline
 
 import (
+	"fmt"
 	"os"
 	"os/exec"
 	"runtime"
 	"strings"
 )
 
-func runEditor(content string, defaultEditor string) (string, error) {
+const (
+	defaultEditor = "vi"
+	defaultShell  = "/bin/bash"
+	windowsEditor = "notepad"
+	windowsShell  = "cmd"
+)
+
+func platformize(linux, windows string) string {
+	if runtime.GOOS == "windows" {
+		return windows
+	}
+	return linux
+}
+
+func defaultEnvShell() []string {
+	shell := os.Getenv("SHELL")
+	if len(shell) == 0 {
+		shell = platformize(defaultShell, windowsShell)
+	}
+	flag := "-c"
+	if shell == windowsShell {
+		flag = "/C"
+	}
+	return []string{shell, flag}
+}
+
+func resolveEditor() ([]string, bool) {
+	editor := strings.TrimSpace(os.Getenv("EDITOR"))
+	if len(editor) == 0 {
+		editor = platformize(defaultEditor, windowsEditor)
+	}
+
+	if !strings.Contains(editor, " ") {
+		return []string{editor}, false
+	}
+
+	if !strings.ContainsAny(editor, "\"'\\") {
+		return strings.Split(editor, " "), false
+	}
+
+	shell := defaultEnvShell()
+	return append(shell, editor), true
+}
+
+func buildEditorCmd(filePath string) *exec.Cmd {
+	args, shell := resolveEditor()
+
+	if shell {
+		// The editor string is the last element — append the file path to it
+		// so the shell interprets the full command.
+		args[len(args)-1] = fmt.Sprintf("%s %s", args[len(args)-1], filePath)
+	} else {
+		args = append(args, filePath)
+	}
+
+	cmd := exec.Command(args[0], args[1:]...)
+	cmd.Stdin = os.Stdin
+	cmd.Stdout = os.Stdout
+	cmd.Stderr = os.Stderr
+	return cmd
+}
+
+func runEditor(content string) (string, error) {
 	tmpFile, err := os.CreateTemp("", "docker-model-prompt-*.txt")
 	if err != nil {
 		return content, err
@@ -20,7 +83,7 @@ func runEditor(content string, defaultEditor string) (string, error) {
 	}
 	tmpFile.Close()
 
-	cmd := buildEditorCmd(defaultEditor, tmpFile.Name())
+	cmd := buildEditorCmd(tmpFile.Name())
 	if err := cmd.Run(); err != nil {
 		return content, err
 	}
@@ -30,25 +93,5 @@ func runEditor(content string, defaultEditor string) (string, error) {
 		return content, err
 	}
 
-	result := strings.TrimRight(string(edited), "\r\n")
-
-	return result, nil
-}
-
-func buildEditorCmd(defaultEditor string, filePath string) *exec.Cmd {
-	editor := strings.TrimSpace(os.Getenv("EDITOR"))
-	if editor == "" {
-		editor = defaultEditor
-	}
-
-	var cmd *exec.Cmd
-	if runtime.GOOS == "windows" {
-		cmd = exec.Command("cmd", "/C", editor+" \""+filePath+"\"")
-	} else {
-		cmd = exec.Command("sh", "-c", editor+" \"$1\"", "--", filePath)
-	}
-	cmd.Stdin = os.Stdin
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = os.Stderr
-	return cmd
+	return strings.TrimRight(string(edited), "\r\n"), nil
 }

cmd/cli/readline/editor_test.go

@@ -55,7 +55,7 @@ func TestRunEditor(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			createMockEditor(t, tt.mockEditorScript)
 
-			result, err := runEditor(tt.input, "vi")
+			result, err := runEditor(tt.input)
 			if err != nil {
 				t.Fatalf("runEditor failed: %v", err)
 			}
@@ -71,7 +71,7 @@ func TestRunEditorReturnsOriginalContentOnFailure(t *testing.T) {
 	t.Setenv("EDITOR", "non_exists_editor")
 
 	content := "docker model prompt hello"
-	result, err := runEditor(content, "vi")
+	result, err := runEditor(content)
 	if err == nil {
 		t.Fatal("expected error from nonexistent editor")
 	}
@@ -85,7 +85,7 @@ func TestRunEditorWithEditorArgs(t *testing.T) {
 	editorScript := createMockEditor(t, `printf "edited with args" > "$2"`)
 	t.Setenv("EDITOR", editorScript+" --wait")
 
-	result, err := runEditor("original", "vi")
+	result, err := runEditor("original")
 	if err != nil {
 		t.Fatalf("runEditor failed: %v", err)
 	}

cmd/cli/readline/readline_unix.go

@@ -25,10 +25,8 @@ func openInEditor(fd uintptr, termios any, content string) (string, error) {
 		return content, err
 	}
 
-	edited, err := runEditor(content, "vi")
+	edited, err := runEditor(content)
 
-	// Always restore raw mode using the original termios, whether the editor
-	// succeeded or failed, so the terminal returns to its previous configuration.
 	if _, restoreErr := SetRawMode(fd); restoreErr != nil {
 		return content, restoreErr
 	}

cmd/cli/readline/readline_windows.go

@@ -12,10 +12,8 @@ func openInEditor(fd uintptr, termios any, content string) (string, error) {
 		return content, err
 	}
 
-	edited, err := runEditor(content, "notepad")
+	edited, err := runEditor(content)
 
-	// Always restore raw mode using the original state, whether the editor
-	// succeeded or failed, so the terminal returns to its previous configuration.
 	if _, restoreErr := SetRawMode(fd); restoreErr != nil {
 		return content, restoreErr
 	}