From 2d51ccaba0f6ab2b354f847e6cb11524224f6f61 Mon Sep 17 00:00:00 2001 From: joeriddles Date: Tue, 22 Oct 2024 09:51:17 -0700 Subject: [PATCH 1/8] Parse args from EDITOR environment variable when editing context --- cli/context_command.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cli/context_command.go b/cli/context_command.go index af48a417..9c3e0956 100644 --- a/cli/context_command.go +++ b/cli/context_command.go @@ -239,6 +239,10 @@ func (c *ctxCommand) editCommand(pc *fisk.ParseContext) error { return fmt.Errorf("set EDITOR environment variable to your chosen editor") } + editorAndArgs := strings.Fields(editor) + editor = editorAndArgs[0] + editorArgs := editorAndArgs[1:] + if !natscontext.IsKnown(c.name) { return fmt.Errorf("unknown context %q", c.name) } @@ -289,7 +293,8 @@ func (c *ctxCommand) editCommand(pc *fisk.ParseContext) error { editFile = f.Name() } - cmd := exec.Command(editor, editFile) + args := append(editorArgs, editFile) + cmd := exec.Command(editor, args...) cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr From b120d8f63ce297276783ae57215d8474ebce1255 Mon Sep 17 00:00:00 2001 From: joeriddles Date: Tue, 22 Oct 2024 10:40:49 -0700 Subject: [PATCH 2/8] Add splitCommand and use for all EDITOR uses --- cli/consumer_command.go | 4 +++- cli/context_command.go | 7 ++----- cli/errors_command.go | 8 ++++++-- cli/stream_command.go | 4 +++- cli/util.go | 8 ++++++++ cli/util_test.go | 18 ++++++++++++++++++ 6 files changed, 40 insertions(+), 9 deletions(-) diff --git a/cli/consumer_command.go b/cli/consumer_command.go index e1dcd859..215e616e 100644 --- a/cli/consumer_command.go +++ b/cli/consumer_command.go @@ -565,6 +565,7 @@ func (c *consumerCmd) interactiveEdit(cfg api.ConsumerConfig) (*api.ConsumerConf if editor == "" { return &api.ConsumerConfig{}, fmt.Errorf("set EDITOR environment variable to your chosen editor") } + editor, args := splitCommand(editor) cj, err := decoratedYamlMarshal(cfg) if err != nil { @@ -584,7 +585,8 @@ func (c *consumerCmd) interactiveEdit(cfg api.ConsumerConfig) (*api.ConsumerConf tfile.Close() - cmd := exec.Command(editor, tfile.Name()) + args = append(args, tfile.Name()) + cmd := exec.Command(editor, args...) cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/cli/context_command.go b/cli/context_command.go index 9c3e0956..92b6b883 100644 --- a/cli/context_command.go +++ b/cli/context_command.go @@ -238,10 +238,7 @@ func (c *ctxCommand) editCommand(pc *fisk.ParseContext) error { if editor == "" { return fmt.Errorf("set EDITOR environment variable to your chosen editor") } - - editorAndArgs := strings.Fields(editor) - editor = editorAndArgs[0] - editorArgs := editorAndArgs[1:] + editor, args := splitCommand(editor) if !natscontext.IsKnown(c.name) { return fmt.Errorf("unknown context %q", c.name) @@ -293,7 +290,7 @@ func (c *ctxCommand) editCommand(pc *fisk.ParseContext) error { editFile = f.Name() } - args := append(editorArgs, editFile) + args = append(args, editFile) cmd := exec.Command(editor, args...) cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout diff --git a/cli/errors_command.go b/cli/errors_command.go index a8c05d44..00104fc2 100644 --- a/cli/errors_command.go +++ b/cli/errors_command.go @@ -122,9 +122,11 @@ func (c *errCmd) listAction(_ *fisk.ParseContext) error { } func (c *errCmd) editAction(pc *fisk.ParseContext) error { - if os.Getenv("EDITOR") == "" { + editor := os.Getenv("EDITOR") + if editor == "" { return fmt.Errorf("EDITOR variable is not set") } + editor, args := splitCommand(editor) errs, err := c.loadErrors(nil) if err != nil { @@ -162,8 +164,10 @@ func (c *errCmd) editAction(pc *fisk.ParseContext) error { tfile.Write(fj) tfile.Close() + args = append(args, tfile.Name()) + for { - cmd := exec.Command(os.Getenv("EDITOR"), tfile.Name()) + cmd := exec.Command(editor, args...) cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/cli/stream_command.go b/cli/stream_command.go index fa6ca129..b1f25f53 100644 --- a/cli/stream_command.go +++ b/cli/stream_command.go @@ -1797,6 +1797,7 @@ func (c *streamCmd) interactiveEdit(cfg api.StreamConfig) (api.StreamConfig, err if editor == "" { return api.StreamConfig{}, fmt.Errorf("set EDITOR environment variable to your chosen editor") } + editor, args := splitCommand(editor) cj, err := decoratedYamlMarshal(cfg) if err != nil { @@ -1816,7 +1817,8 @@ func (c *streamCmd) interactiveEdit(cfg api.StreamConfig) (api.StreamConfig, err tfile.Close() - cmd := exec.Command(editor, tfile.Name()) + args = append(args, tfile.Name()) + cmd := exec.Command(editor, args...) cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/cli/util.go b/cli/util.go index 4f00a223..75607b0e 100644 --- a/cli/util.go +++ b/cli/util.go @@ -254,6 +254,14 @@ func splitString(s string) []string { }) } +// Split the string into a command and its arguments. +func splitCommand(s string) (string, []string) { + cmdAndArgs := strings.Fields(s) + cmd := cmdAndArgs[0] + args := cmdAndArgs[1:] + return cmd, args +} + func splitCLISubjects(subjects []string) []string { new := []string{} diff --git a/cli/util_test.go b/cli/util_test.go index 3a25cce9..f0e5a371 100644 --- a/cli/util_test.go +++ b/cli/util_test.go @@ -15,6 +15,7 @@ package cli import ( "errors" + "slices" "testing" "github.com/google/go-cmp/cmp" @@ -85,6 +86,23 @@ func TestSplitString(t *testing.T) { } } +func TestSplitCommand(t *testing.T) { + cmd, args := splitCommand("vim") + if cmd != "vim" && len(args) != 0 { + t.Fatalf("Expected vim and [], got %v and %v", cmd, args) + } + + cmd, args = splitCommand("vscode --wait") + if cmd != "vscode" && !slices.Equal(args, []string{"--wait"}) { + t.Fatalf("Expected vscode and [\"--wait\"], got %v and %v", cmd, args) + } + + cmd, args = splitCommand("vscode --wait --new-window") + if cmd != "vscode" && !slices.Equal(args, []string{"--wait", "--new-window"}) { + t.Fatalf("Expected vscode and [\"--wait\", \"--new-window\"], got %v and %v", cmd, args) + } +} + func TestRandomString(t *testing.T) { for i := 0; i < 1000; i++ { if len(randomString(1024, 1024)) != 1024 { From afcbc41d2091f494ba93a10c86d8b37f75980b70 Mon Sep 17 00:00:00 2001 From: joeriddles Date: Tue, 22 Oct 2024 10:44:45 -0700 Subject: [PATCH 3/8] Fix TestSplitCommand to use the correct vscode CLI command --- cli/util_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cli/util_test.go b/cli/util_test.go index f0e5a371..ec8bcdcb 100644 --- a/cli/util_test.go +++ b/cli/util_test.go @@ -92,14 +92,14 @@ func TestSplitCommand(t *testing.T) { t.Fatalf("Expected vim and [], got %v and %v", cmd, args) } - cmd, args = splitCommand("vscode --wait") - if cmd != "vscode" && !slices.Equal(args, []string{"--wait"}) { - t.Fatalf("Expected vscode and [\"--wait\"], got %v and %v", cmd, args) + cmd, args = splitCommand("code --wait") + if cmd != "code" && !slices.Equal(args, []string{"--wait"}) { + t.Fatalf("Expected code and [\"--wait\"], got %v and %v", cmd, args) } - cmd, args = splitCommand("vscode --wait --new-window") - if cmd != "vscode" && !slices.Equal(args, []string{"--wait", "--new-window"}) { - t.Fatalf("Expected vscode and [\"--wait\", \"--new-window\"], got %v and %v", cmd, args) + cmd, args = splitCommand("code --wait --new-window") + if cmd != "code" && !slices.Equal(args, []string{"--wait", "--new-window"}) { + t.Fatalf("Expected code and [\"--wait\", \"--new-window\"], got %v and %v", cmd, args) } } From 48fbe2963604a9af4810410e04a683c03a93b490 Mon Sep 17 00:00:00 2001 From: joeriddles Date: Tue, 22 Oct 2024 15:52:18 -0700 Subject: [PATCH 4/8] Use shlex.Split instead of strings.Fields for shell awareness --- cli/consumer_command.go | 10 +++++++--- cli/context_command.go | 10 +++++++--- cli/errors_command.go | 10 +++++++--- cli/stream_command.go | 10 +++++++--- cli/util.go | 10 +++++++--- cli/util_test.go | 21 ++++++++++++++++++--- 6 files changed, 53 insertions(+), 18 deletions(-) diff --git a/cli/consumer_command.go b/cli/consumer_command.go index 215e616e..9c527ecd 100644 --- a/cli/consumer_command.go +++ b/cli/consumer_command.go @@ -561,11 +561,15 @@ func (c *consumerCmd) leaderStandDownAction(_ *fisk.ParseContext) error { } func (c *consumerCmd) interactiveEdit(cfg api.ConsumerConfig) (*api.ConsumerConfig, error) { - editor := os.Getenv("EDITOR") - if editor == "" { + rawEditor := os.Getenv("EDITOR") + if rawEditor == "" { return &api.ConsumerConfig{}, fmt.Errorf("set EDITOR environment variable to your chosen editor") } - editor, args := splitCommand(editor) + + editor, args, err := splitCommand(rawEditor) + if err != nil { + return &api.ConsumerConfig{}, fmt.Errorf("could not parse EDITOR: %v", rawEditor) + } cj, err := decoratedYamlMarshal(cfg) if err != nil { diff --git a/cli/context_command.go b/cli/context_command.go index 92b6b883..12a4f21a 100644 --- a/cli/context_command.go +++ b/cli/context_command.go @@ -234,11 +234,15 @@ socks_proxy: {{ .SocksProxy | t }} ` func (c *ctxCommand) editCommand(pc *fisk.ParseContext) error { - editor := os.Getenv("EDITOR") - if editor == "" { + rawEditor := os.Getenv("EDITOR") + if rawEditor == "" { return fmt.Errorf("set EDITOR environment variable to your chosen editor") } - editor, args := splitCommand(editor) + + editor, args, err := splitCommand(rawEditor) + if err != nil { + return fmt.Errorf("could not parse EDITOR: %v", rawEditor) + } if !natscontext.IsKnown(c.name) { return fmt.Errorf("unknown context %q", c.name) diff --git a/cli/errors_command.go b/cli/errors_command.go index 00104fc2..99ce351a 100644 --- a/cli/errors_command.go +++ b/cli/errors_command.go @@ -122,11 +122,15 @@ func (c *errCmd) listAction(_ *fisk.ParseContext) error { } func (c *errCmd) editAction(pc *fisk.ParseContext) error { - editor := os.Getenv("EDITOR") - if editor == "" { + rawEditor := os.Getenv("EDITOR") + if rawEditor == "" { return fmt.Errorf("EDITOR variable is not set") } - editor, args := splitCommand(editor) + + editor, args, err := splitCommand(rawEditor) + if err != nil { + return fmt.Errorf("could not parse EDITOR: %v", rawEditor) + } errs, err := c.loadErrors(nil) if err != nil { diff --git a/cli/stream_command.go b/cli/stream_command.go index b1f25f53..771f7598 100644 --- a/cli/stream_command.go +++ b/cli/stream_command.go @@ -1793,11 +1793,15 @@ func (c *streamCmd) copyAndEditStream(cfg api.StreamConfig, pc *fisk.ParseContex } func (c *streamCmd) interactiveEdit(cfg api.StreamConfig) (api.StreamConfig, error) { - editor := os.Getenv("EDITOR") - if editor == "" { + rawEditor := os.Getenv("EDITOR") + if rawEditor == "" { return api.StreamConfig{}, fmt.Errorf("set EDITOR environment variable to your chosen editor") } - editor, args := splitCommand(editor) + + editor, args, err := splitCommand(rawEditor) + if err != nil { + return api.StreamConfig{}, fmt.Errorf("could not parse EDITOR: %v", rawEditor) + } cj, err := decoratedYamlMarshal(cfg) if err != nil { diff --git a/cli/util.go b/cli/util.go index 75607b0e..6e5469ca 100644 --- a/cli/util.go +++ b/cli/util.go @@ -255,11 +255,15 @@ func splitString(s string) []string { } // Split the string into a command and its arguments. -func splitCommand(s string) (string, []string) { - cmdAndArgs := strings.Fields(s) +func splitCommand(s string) (string, []string, error) { + cmdAndArgs, err := shlex.Split(s) + if err != nil { + return "", nil, err + } + cmd := cmdAndArgs[0] args := cmdAndArgs[1:] - return cmd, args + return cmd, args, nil } func splitCLISubjects(subjects []string) []string { diff --git a/cli/util_test.go b/cli/util_test.go index ec8bcdcb..a8341bed 100644 --- a/cli/util_test.go +++ b/cli/util_test.go @@ -87,20 +87,35 @@ func TestSplitString(t *testing.T) { } func TestSplitCommand(t *testing.T) { - cmd, args := splitCommand("vim") + cmd, args, err := splitCommand("vim") + if err != nil { + t.Fatalf("Expected err to be nil, got %v", err) + } if cmd != "vim" && len(args) != 0 { t.Fatalf("Expected vim and [], got %v and %v", cmd, args) } - cmd, args = splitCommand("code --wait") + cmd, args, err = splitCommand("code --wait") + if err != nil { + t.Fatalf("Expected err to be nil, got %v", err) + } if cmd != "code" && !slices.Equal(args, []string{"--wait"}) { t.Fatalf("Expected code and [\"--wait\"], got %v and %v", cmd, args) } - cmd, args = splitCommand("code --wait --new-window") + cmd, args, err = splitCommand("code --wait --new-window") + if err != nil { + t.Fatalf("Expected err to be nil, got %v", err) + } if cmd != "code" && !slices.Equal(args, []string{"--wait", "--new-window"}) { t.Fatalf("Expected code and [\"--wait\", \"--new-window\"], got %v and %v", cmd, args) } + + // EOF found when expecting closing quote + _, _, err = splitCommand("foo --bar 'hello") + if err == nil { + t.Fatalf("Expdected err to not be nil, got %v", err) + } } func TestRandomString(t *testing.T) { From 4092441d50566512d48228faf62ea5756f23efc8 Mon Sep 17 00:00:00 2001 From: joeriddles Date: Mon, 28 Oct 2024 13:41:26 -0700 Subject: [PATCH 5/8] Add editFile util function to CLI --- cli/consumer_command.go | 21 ++------------------- cli/context_command.go | 29 ++++++----------------------- cli/errors_command.go | 22 +++------------------- cli/stream_command.go | 21 ++------------------- cli/util.go | 26 ++++++++++++++++++++++++++ 5 files changed, 39 insertions(+), 80 deletions(-) diff --git a/cli/consumer_command.go b/cli/consumer_command.go index 9c527ecd..06707ec4 100644 --- a/cli/consumer_command.go +++ b/cli/consumer_command.go @@ -22,7 +22,6 @@ import ( "math" "math/rand" "os" - "os/exec" "os/signal" "regexp" "sort" @@ -561,16 +560,6 @@ func (c *consumerCmd) leaderStandDownAction(_ *fisk.ParseContext) error { } func (c *consumerCmd) interactiveEdit(cfg api.ConsumerConfig) (*api.ConsumerConfig, error) { - rawEditor := os.Getenv("EDITOR") - if rawEditor == "" { - return &api.ConsumerConfig{}, fmt.Errorf("set EDITOR environment variable to your chosen editor") - } - - editor, args, err := splitCommand(rawEditor) - if err != nil { - return &api.ConsumerConfig{}, fmt.Errorf("could not parse EDITOR: %v", rawEditor) - } - cj, err := decoratedYamlMarshal(cfg) if err != nil { return &api.ConsumerConfig{}, fmt.Errorf("could not create temporary file: %s", err) @@ -589,15 +578,9 @@ func (c *consumerCmd) interactiveEdit(cfg api.ConsumerConfig) (*api.ConsumerConf tfile.Close() - args = append(args, tfile.Name()) - cmd := exec.Command(editor, args...) - cmd.Stdin = os.Stdin - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - - err = cmd.Run() + err = editFile(tfile.Name()) if err != nil { - return &api.ConsumerConfig{}, fmt.Errorf("could not create temporary file: %s", err) + return &api.ConsumerConfig{}, err } nb, err := os.ReadFile(tfile.Name()) diff --git a/cli/context_command.go b/cli/context_command.go index 12a4f21a..4749a17e 100644 --- a/cli/context_command.go +++ b/cli/context_command.go @@ -18,7 +18,6 @@ import ( "encoding/json" "fmt" "os" - "os/exec" "os/user" "sort" "strings" @@ -234,16 +233,6 @@ socks_proxy: {{ .SocksProxy | t }} ` func (c *ctxCommand) editCommand(pc *fisk.ParseContext) error { - rawEditor := os.Getenv("EDITOR") - if rawEditor == "" { - return fmt.Errorf("set EDITOR environment variable to your chosen editor") - } - - editor, args, err := splitCommand(rawEditor) - if err != nil { - return fmt.Errorf("could not parse EDITOR: %v", rawEditor) - } - if !natscontext.IsKnown(c.name) { return fmt.Errorf("unknown context %q", c.name) } @@ -252,7 +241,7 @@ func (c *ctxCommand) editCommand(pc *fisk.ParseContext) error { if err != nil { return err } - editFile := path + editFp := path var ctx *natscontext.Context @@ -291,22 +280,16 @@ func (c *ctxCommand) editCommand(pc *fisk.ParseContext) error { } f.Close() - editFile = f.Name() + editFp = f.Name() } - args = append(args, editFile) - cmd := exec.Command(editor, args...) - cmd.Stdin = os.Stdin - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - - err = cmd.Run() + err = editFile(editFp) if err != nil { return err } - if path != editFile { - yctx, err := os.ReadFile(editFile) + if path != editFp { + yctx, err := os.ReadFile(editFp) if err != nil { return fmt.Errorf("could not read temporary copy: %w", err) } @@ -338,7 +321,7 @@ func (c *ctxCommand) editCommand(pc *fisk.ParseContext) error { err = c.showCommand(pc) if err != nil { // but not if the file was already corrupt and we are editing the json directly - if path == editFile { + if path == editFp { return err } diff --git a/cli/errors_command.go b/cli/errors_command.go index 99ce351a..ac1f1b3c 100644 --- a/cli/errors_command.go +++ b/cli/errors_command.go @@ -18,7 +18,6 @@ import ( "errors" "fmt" "os" - "os/exec" "regexp" "sort" "strconv" @@ -122,16 +121,6 @@ func (c *errCmd) listAction(_ *fisk.ParseContext) error { } func (c *errCmd) editAction(pc *fisk.ParseContext) error { - rawEditor := os.Getenv("EDITOR") - if rawEditor == "" { - return fmt.Errorf("EDITOR variable is not set") - } - - editor, args, err := splitCommand(rawEditor) - if err != nil { - return fmt.Errorf("could not parse EDITOR: %v", rawEditor) - } - errs, err := c.loadErrors(nil) if err != nil { return err @@ -168,17 +157,12 @@ func (c *errCmd) editAction(pc *fisk.ParseContext) error { tfile.Write(fj) tfile.Close() - args = append(args, tfile.Name()) + fp := tfile.Name() for { - cmd := exec.Command(editor, args...) - cmd.Stdin = os.Stdin - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - - err = cmd.Run() + err = editFile(fp) if err != nil { - return fmt.Errorf("could not edit error: %s", err) + return err } eb, err := os.ReadFile(tfile.Name()) diff --git a/cli/stream_command.go b/cli/stream_command.go index 771f7598..8c2d9bd1 100644 --- a/cli/stream_command.go +++ b/cli/stream_command.go @@ -21,7 +21,6 @@ import ( "io" "math" "os" - "os/exec" "os/signal" "path/filepath" "sort" @@ -1793,16 +1792,6 @@ func (c *streamCmd) copyAndEditStream(cfg api.StreamConfig, pc *fisk.ParseContex } func (c *streamCmd) interactiveEdit(cfg api.StreamConfig) (api.StreamConfig, error) { - rawEditor := os.Getenv("EDITOR") - if rawEditor == "" { - return api.StreamConfig{}, fmt.Errorf("set EDITOR environment variable to your chosen editor") - } - - editor, args, err := splitCommand(rawEditor) - if err != nil { - return api.StreamConfig{}, fmt.Errorf("could not parse EDITOR: %v", rawEditor) - } - cj, err := decoratedYamlMarshal(cfg) if err != nil { return api.StreamConfig{}, fmt.Errorf("could not create temporary file: %s", err) @@ -1821,15 +1810,9 @@ func (c *streamCmd) interactiveEdit(cfg api.StreamConfig) (api.StreamConfig, err tfile.Close() - args = append(args, tfile.Name()) - cmd := exec.Command(editor, args...) - cmd.Stdin = os.Stdin - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - - err = cmd.Run() + err = editFile(tfile.Name()) if err != nil { - return api.StreamConfig{}, fmt.Errorf("could not create temporary file: %s", err) + return api.StreamConfig{}, err } nb, err := os.ReadFile(tfile.Name()) diff --git a/cli/util.go b/cli/util.go index 6e5469ca..0bc79bc4 100644 --- a/cli/util.go +++ b/cli/util.go @@ -281,6 +281,32 @@ func splitCLISubjects(subjects []string) []string { return new } +// Edit the file at filepath f. +func editFile(f string) error { + rawEditor := os.Getenv("EDITOR") + if rawEditor == "" { + return fmt.Errorf("set EDITOR environment variable to your chosen editor") + } + + editor, args, err := splitCommand(rawEditor) + if err != nil { + return fmt.Errorf("could not parse EDITOR: %v", rawEditor) + } + + args = append(args, f) + cmd := exec.Command(editor, args...) + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + err = cmd.Run() + if err != nil { + return fmt.Errorf("could not edit file %v: %s", f, err) + } + + return nil +} + func natsOpts() []nats.Option { if opts().Config == nil { return []nats.Option{} From 0d162c2a8ab1db53711aa34a3cf097f3f164c4e5 Mon Sep 17 00:00:00 2001 From: joeriddles Date: Mon, 28 Oct 2024 14:04:02 -0700 Subject: [PATCH 6/8] Add TestEditFile to test CLI util editFile --- cli/util_test.go | 53 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/cli/util_test.go b/cli/util_test.go index a8341bed..061f11ee 100644 --- a/cli/util_test.go +++ b/cli/util_test.go @@ -15,7 +15,10 @@ package cli import ( "errors" + "io" + "os" "slices" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -114,10 +117,58 @@ func TestSplitCommand(t *testing.T) { // EOF found when expecting closing quote _, _, err = splitCommand("foo --bar 'hello") if err == nil { - t.Fatalf("Expdected err to not be nil, got %v", err) + t.Fatal("Expected err to not be nil, got nil") } } +func TestEditFile(t *testing.T) { + r, w, _ := os.Pipe() + os.Stdout = w + defer r.Close() + defer w.Close() + + f, err := os.CreateTemp("", "test_edit_file") + if err != nil { + t.Fatalf("Expected err to be nil, got %v", err) + } + defer f.Close() + defer os.Remove(f.Name()) + + t.Run("EDITOR unset", func(t *testing.T) { + os.Unsetenv("EDITOR") + err := editFile("") + if err == nil { + t.Fatal("Expected err to not be nil, got nil") + } + }) + + t.Run("EDITOR set", func(t *testing.T) { + os.Setenv("EDITOR", "echo") + err := editFile(f.Name()) + if err != nil { + t.Fatalf("Expected err to be nil, got %v", err) + } + + w.Close() + stdout, err := io.ReadAll(r) + if err != nil { + t.Fatalf("Expected err to be nil, got %v", err) + } + r.Close() + + actual := string(stdout) + lines := strings.Split(actual, "\n") + + if len(lines) != 2 || lines[1] != "" { + t.Fatalf("Expeced one line of output, got %v", actual) + } + + if !strings.Contains(lines[0], "test_edit_file") { + t.Fatalf("Expected echo output, got %v", actual) + } + }) +} + func TestRandomString(t *testing.T) { for i := 0; i < 1000; i++ { if len(randomString(1024, 1024)) != 1024 { From 7acdaca7f48c43178e8c85d93c62d89b9856bee3 Mon Sep 17 00:00:00 2001 From: joeriddles Date: Tue, 29 Oct 2024 07:29:00 -0700 Subject: [PATCH 7/8] Fix misspelling of expected --- cli/util_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/util_test.go b/cli/util_test.go index 061f11ee..14f1a726 100644 --- a/cli/util_test.go +++ b/cli/util_test.go @@ -160,7 +160,7 @@ func TestEditFile(t *testing.T) { lines := strings.Split(actual, "\n") if len(lines) != 2 || lines[1] != "" { - t.Fatalf("Expeced one line of output, got %v", actual) + t.Fatalf("Expected one line of output, got %v", actual) } if !strings.Contains(lines[0], "test_edit_file") { From 8b796553ccd0dc09f9ee49d6462c72fb7e69663e Mon Sep 17 00:00:00 2001 From: joeriddles Date: Tue, 29 Oct 2024 07:33:09 -0700 Subject: [PATCH 8/8] Move new SplitCommand and EditFile util functions to internal/util --- cli/consumer_command.go | 2 +- cli/context_command.go | 2 +- cli/errors_command.go | 3 +- cli/stream_command.go | 2 +- cli/util.go | 38 ----------------- cli/util_test.go | 84 -------------------------------------- internal/util/util.go | 40 ++++++++++++++++++ internal/util/util_test.go | 84 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 129 insertions(+), 126 deletions(-) diff --git a/cli/consumer_command.go b/cli/consumer_command.go index 06707ec4..d36004e1 100644 --- a/cli/consumer_command.go +++ b/cli/consumer_command.go @@ -578,7 +578,7 @@ func (c *consumerCmd) interactiveEdit(cfg api.ConsumerConfig) (*api.ConsumerConf tfile.Close() - err = editFile(tfile.Name()) + err = iu.EditFile(tfile.Name()) if err != nil { return &api.ConsumerConfig{}, err } diff --git a/cli/context_command.go b/cli/context_command.go index 4749a17e..355d810c 100644 --- a/cli/context_command.go +++ b/cli/context_command.go @@ -283,7 +283,7 @@ func (c *ctxCommand) editCommand(pc *fisk.ParseContext) error { editFp = f.Name() } - err = editFile(editFp) + err = iu.EditFile(editFp) if err != nil { return err } diff --git a/cli/errors_command.go b/cli/errors_command.go index ac1f1b3c..dbc4314f 100644 --- a/cli/errors_command.go +++ b/cli/errors_command.go @@ -27,6 +27,7 @@ import ( "github.com/fatih/color" "github.com/nats-io/jsm.go/schemas" "github.com/nats-io/nats-server/v2/server" + iu "github.com/nats-io/natscli/internal/util" ) type errCmd struct { @@ -160,7 +161,7 @@ func (c *errCmd) editAction(pc *fisk.ParseContext) error { fp := tfile.Name() for { - err = editFile(fp) + err = iu.EditFile(fp) if err != nil { return err } diff --git a/cli/stream_command.go b/cli/stream_command.go index 8c2d9bd1..b25becdd 100644 --- a/cli/stream_command.go +++ b/cli/stream_command.go @@ -1810,7 +1810,7 @@ func (c *streamCmd) interactiveEdit(cfg api.StreamConfig) (api.StreamConfig, err tfile.Close() - err = editFile(tfile.Name()) + err = iu.EditFile(tfile.Name()) if err != nil { return api.StreamConfig{}, err } diff --git a/cli/util.go b/cli/util.go index 0bc79bc4..4f00a223 100644 --- a/cli/util.go +++ b/cli/util.go @@ -254,18 +254,6 @@ func splitString(s string) []string { }) } -// Split the string into a command and its arguments. -func splitCommand(s string) (string, []string, error) { - cmdAndArgs, err := shlex.Split(s) - if err != nil { - return "", nil, err - } - - cmd := cmdAndArgs[0] - args := cmdAndArgs[1:] - return cmd, args, nil -} - func splitCLISubjects(subjects []string) []string { new := []string{} @@ -281,32 +269,6 @@ func splitCLISubjects(subjects []string) []string { return new } -// Edit the file at filepath f. -func editFile(f string) error { - rawEditor := os.Getenv("EDITOR") - if rawEditor == "" { - return fmt.Errorf("set EDITOR environment variable to your chosen editor") - } - - editor, args, err := splitCommand(rawEditor) - if err != nil { - return fmt.Errorf("could not parse EDITOR: %v", rawEditor) - } - - args = append(args, f) - cmd := exec.Command(editor, args...) - cmd.Stdin = os.Stdin - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - - err = cmd.Run() - if err != nil { - return fmt.Errorf("could not edit file %v: %s", f, err) - } - - return nil -} - func natsOpts() []nats.Option { if opts().Config == nil { return []nats.Option{} diff --git a/cli/util_test.go b/cli/util_test.go index 14f1a726..3a25cce9 100644 --- a/cli/util_test.go +++ b/cli/util_test.go @@ -15,10 +15,6 @@ package cli import ( "errors" - "io" - "os" - "slices" - "strings" "testing" "github.com/google/go-cmp/cmp" @@ -89,86 +85,6 @@ func TestSplitString(t *testing.T) { } } -func TestSplitCommand(t *testing.T) { - cmd, args, err := splitCommand("vim") - if err != nil { - t.Fatalf("Expected err to be nil, got %v", err) - } - if cmd != "vim" && len(args) != 0 { - t.Fatalf("Expected vim and [], got %v and %v", cmd, args) - } - - cmd, args, err = splitCommand("code --wait") - if err != nil { - t.Fatalf("Expected err to be nil, got %v", err) - } - if cmd != "code" && !slices.Equal(args, []string{"--wait"}) { - t.Fatalf("Expected code and [\"--wait\"], got %v and %v", cmd, args) - } - - cmd, args, err = splitCommand("code --wait --new-window") - if err != nil { - t.Fatalf("Expected err to be nil, got %v", err) - } - if cmd != "code" && !slices.Equal(args, []string{"--wait", "--new-window"}) { - t.Fatalf("Expected code and [\"--wait\", \"--new-window\"], got %v and %v", cmd, args) - } - - // EOF found when expecting closing quote - _, _, err = splitCommand("foo --bar 'hello") - if err == nil { - t.Fatal("Expected err to not be nil, got nil") - } -} - -func TestEditFile(t *testing.T) { - r, w, _ := os.Pipe() - os.Stdout = w - defer r.Close() - defer w.Close() - - f, err := os.CreateTemp("", "test_edit_file") - if err != nil { - t.Fatalf("Expected err to be nil, got %v", err) - } - defer f.Close() - defer os.Remove(f.Name()) - - t.Run("EDITOR unset", func(t *testing.T) { - os.Unsetenv("EDITOR") - err := editFile("") - if err == nil { - t.Fatal("Expected err to not be nil, got nil") - } - }) - - t.Run("EDITOR set", func(t *testing.T) { - os.Setenv("EDITOR", "echo") - err := editFile(f.Name()) - if err != nil { - t.Fatalf("Expected err to be nil, got %v", err) - } - - w.Close() - stdout, err := io.ReadAll(r) - if err != nil { - t.Fatalf("Expected err to be nil, got %v", err) - } - r.Close() - - actual := string(stdout) - lines := strings.Split(actual, "\n") - - if len(lines) != 2 || lines[1] != "" { - t.Fatalf("Expected one line of output, got %v", actual) - } - - if !strings.Contains(lines[0], "test_edit_file") { - t.Fatalf("Expected echo output, got %v", actual) - } - }) -} - func TestRandomString(t *testing.T) { for i := 0; i < 1000; i++ { if len(randomString(1024, 1024)) != 1024 { diff --git a/internal/util/util.go b/internal/util/util.go index b6d25732..2de121ce 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -21,6 +21,7 @@ import ( "io" "math" "os" + "os/exec" "reflect" "regexp" "sort" @@ -29,6 +30,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/dustin/go-humanize" + "github.com/google/shlex" "github.com/guptarohit/asciigraph" "github.com/nats-io/jsm.go" "github.com/nats-io/nats.go" @@ -430,3 +432,41 @@ func ProgressWidth() int { func JSONString(s string) string { return "\"" + s + "\"" } + +// Split the string into a command and its arguments. +func SplitCommand(s string) (string, []string, error) { + cmdAndArgs, err := shlex.Split(s) + if err != nil { + return "", nil, err + } + + cmd := cmdAndArgs[0] + args := cmdAndArgs[1:] + return cmd, args, nil +} + +// Edit the file at filepath f using the environment variable EDITOR command. +func EditFile(f string) error { + rawEditor := os.Getenv("EDITOR") + if rawEditor == "" { + return fmt.Errorf("set EDITOR environment variable to your chosen editor") + } + + editor, args, err := SplitCommand(rawEditor) + if err != nil { + return fmt.Errorf("could not parse EDITOR: %v", rawEditor) + } + + args = append(args, f) + cmd := exec.Command(editor, args...) + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + err = cmd.Run() + if err != nil { + return fmt.Errorf("could not edit file %v: %s", f, err) + } + + return nil +} diff --git a/internal/util/util_test.go b/internal/util/util_test.go index 24c02db1..402d2169 100644 --- a/internal/util/util_test.go +++ b/internal/util/util_test.go @@ -14,6 +14,10 @@ package util import ( + "io" + "os" + "slices" + "strings" "testing" ) @@ -34,3 +38,83 @@ func TestMultipleSort(t *testing.T) { t.Fatalf("expected true") } } + +func TestSplitCommand(t *testing.T) { + cmd, args, err := SplitCommand("vim") + if err != nil { + t.Fatalf("Expected err to be nil, got %v", err) + } + if cmd != "vim" && len(args) != 0 { + t.Fatalf("Expected vim and [], got %v and %v", cmd, args) + } + + cmd, args, err = SplitCommand("code --wait") + if err != nil { + t.Fatalf("Expected err to be nil, got %v", err) + } + if cmd != "code" && !slices.Equal(args, []string{"--wait"}) { + t.Fatalf("Expected code and [\"--wait\"], got %v and %v", cmd, args) + } + + cmd, args, err = SplitCommand("code --wait --new-window") + if err != nil { + t.Fatalf("Expected err to be nil, got %v", err) + } + if cmd != "code" && !slices.Equal(args, []string{"--wait", "--new-window"}) { + t.Fatalf("Expected code and [\"--wait\", \"--new-window\"], got %v and %v", cmd, args) + } + + // EOF found when expecting closing quote + _, _, err = SplitCommand("foo --bar 'hello") + if err == nil { + t.Fatal("Expected err to not be nil, got nil") + } +} + +func TestEditFile(t *testing.T) { + r, w, _ := os.Pipe() + os.Stdout = w + defer r.Close() + defer w.Close() + + f, err := os.CreateTemp("", "test_edit_file") + if err != nil { + t.Fatalf("Expected err to be nil, got %v", err) + } + defer f.Close() + defer os.Remove(f.Name()) + + t.Run("EDITOR unset", func(t *testing.T) { + os.Unsetenv("EDITOR") + err := EditFile("") + if err == nil { + t.Fatal("Expected err to not be nil, got nil") + } + }) + + t.Run("EDITOR set", func(t *testing.T) { + os.Setenv("EDITOR", "echo") + err := EditFile(f.Name()) + if err != nil { + t.Fatalf("Expected err to be nil, got %v", err) + } + + w.Close() + stdout, err := io.ReadAll(r) + if err != nil { + t.Fatalf("Expected err to be nil, got %v", err) + } + r.Close() + + actual := string(stdout) + lines := strings.Split(actual, "\n") + + if len(lines) != 2 || lines[1] != "" { + t.Fatalf("Expected one line of output, got %v", actual) + } + + if !strings.Contains(lines[0], "test_edit_file") { + t.Fatalf("Expected echo output, got %v", actual) + } + }) +}