cli: env edit fixes (#42)

- Do not lose edits when the new YAML contains errors
- Automatically append "-w" to the editor if it is `code`
- Default to `vi` if no other editor is present
- Fix the help text to remove the term 'open'

Fixes #25.
Fixes #27.

cmd/esc/cli/cli_test.go

@@ -480,7 +480,7 @@ func (c *testExec) LookPath(cmd string) (string, error) {
 }
 
 func (c *testExec) Run(cmd *exec.Cmd) error {
-	script, ok := c.commands[cmd.Path]
+	script, ok := c.commands[filepath.Base(cmd.Path)]
 	if !ok {
 		return errors.New("command not found")
 	}
@@ -505,6 +505,9 @@ func (c *testExec) Run(cmd *exec.Cmd) error {
 				f = &fstest.MapFile{Mode: perm}
 				c.fs.MapFS[path] = &fstest.MapFile{Mode: perm}
 			}
+			if flag&os.O_TRUNC != 0 {
+				f.Data = f.Data[:0]
+			}
 			return &testFile{f: f}, nil
 		}),
 		interp.ReadDirHandler(func(ctx context.Context, path string) ([]os.FileInfo, error) {
@@ -515,7 +518,7 @@ func (c *testExec) Run(cmd *exec.Cmd) error {
 		}),
 		interp.StdIO(cmd.Stdin, cmd.Stdout, cmd.Stderr),
 		interp.Env(expand.ListEnviron(cmd.Env...)),
-		interp.Params(cmd.Args[1:]...),
+		interp.Params(append([]string{"--"}, cmd.Args[1:]...)...),
 	)
 	if err != nil {
 		return err
@@ -677,6 +680,8 @@ func TestCLI(t *testing.T) {
 			if accept() {
 				if err != nil {
 					def.Error = err.Error()
+				} else {
+					def.Error = ""
 				}
 
 				def.Stdout = testcase.stdout.String()

cmd/esc/cli/env_edit.go

@@ -8,6 +8,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"io/fs"
 	"os/exec"
 	"strings"
@@ -16,6 +17,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/pulumi/esc"
+	"github.com/pulumi/esc/cmd/esc/cli/client"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
 )
 
@@ -31,13 +33,15 @@ func newEnvEditCmd(env *envCommand) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "edit [<org-name>/]<environment-name>",
 		Args:  cobra.MaximumNArgs(1),
-		Short: "Open an environment for editing.",
-		Long: "Open an environment for editing\n" +
+		Short: "Edit an environment definition",
+		Long: "Edit an environment definition\n" +
 			"\n" +
 			"This command fetches the current definition for the named environment and opens it\n" +
 			"for editing in an editor. The editor defaults to the value of the VISUAL environment\n" +
 			"variable. If VISUAL is not set, EDITOR is used. These values are interpreted as\n" +
-			"commands to which the name of the temporary file used for the environment is appended.\n",
+			"commands to which the name of the temporary file used for the environment is appended.\n" +
+			"If no editor is specified via the --editor flag or environment variables, edit\n" +
+			"defaults to `vi`.\n",
 		SilenceUsage: true,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			ctx := context.Background()
@@ -62,40 +66,47 @@ func newEnvEditCmd(env *envCommand) *cobra.Command {
 				return fmt.Errorf("getting environment definition: %w", err)
 			}
 
-			var checked []byte
+			var env *esc.Environment
+			var diags []client.EnvironmentDiagnostic
 			if len(yaml) != 0 {
-				env, diags, err := edit.env.esc.client.CheckYAMLEnvironment(ctx, orgName, yaml)
+				env, diags, _ = edit.env.esc.client.CheckYAMLEnvironment(ctx, orgName, yaml)
+			}
+
+			for {
+				newYAML, err := edit.editWithYAMLEditor(editor, envName, yaml, env, diags)
 				if err != nil {
-					checked = []byte(fmt.Sprintf("# checking environment: %v\n", err))
-				} else if len(diags) != 0 {
-					var stderr bytes.Buffer
-					err = edit.env.writeYAMLEnvironmentDiagnostics(&stderr, envName, yaml, diags)
-					contract.IgnoreError(err)
-					checked = stderr.Bytes()
-				} else {
-					var stdout bytes.Buffer
-					enc := json.NewEncoder(&stdout)
-					enc.SetIndent("", "  ")
-					err = enc.Encode(esc.NewValue(env.Properties).ToJSON(false))
-					contract.IgnoreError(err)
-					checked = stdout.Bytes()
+					return err
+				}
+				if len(bytes.TrimSpace(newYAML)) == 0 {
+					fmt.Fprintln(edit.env.esc.stderr, "Aborting edit due to empty definition.")
+					return nil
 				}
-			}
 
-			newYAML, err := edit.editWithYAMLEditor(editor, yaml, checked)
-			if err != nil {
-				return err
-			}
+				diags, err = edit.env.esc.client.UpdateEnvironment(ctx, orgName, envName, newYAML, tag)
+				if err != nil {
+					return fmt.Errorf("updating environment definition: %w", err)
+				}
+				if len(diags) == 0 {
+					fmt.Fprintln(edit.env.esc.stdout, "Environment updated.")
+					return nil
+				}
 
-			diags, err := edit.env.esc.client.UpdateEnvironment(ctx, orgName, envName, newYAML, tag)
-			if err != nil {
-				return fmt.Errorf("updating environment definition: %w", err)
-			}
-			if len(diags) != 0 {
-				return edit.env.writeYAMLEnvironmentDiagnostics(edit.env.esc.stderr, envName, newYAML, diags)
-			}
+				err = edit.env.writeYAMLEnvironmentDiagnostics(edit.env.esc.stderr, envName, newYAML, diags)
+				contract.IgnoreError(err)
 
-			return nil
+				fmt.Fprintln(edit.env.esc.stderr, "Press ENTER to continue editing or ^D to exit")
+
+				var b [1]byte
+				if _, err := edit.env.esc.stdin.Read(b[:]); err != nil {
+					if errors.Is(err, io.EOF) {
+						fmt.Fprintln(edit.env.esc.stderr, "Aborting edit.")
+						return nil
+					}
+					return err
+				}
+
+				yaml = newYAML
+			}
 		},
 	}
 
@@ -147,8 +158,18 @@ func (edit *envEditCommand) getEditor() ([]string, error) {
 		}
 	}
 	if len(args) == 0 {
-		return nil, errors.New("No available editor. Please use the --editor flag or set one of the " +
-			"VISUAL or EDITOR environment variables.")
+		path, err := edit.env.esc.exec.LookPath("vi")
+		if err != nil {
+			return nil, errors.New("No available editor. Please use the --editor flag or set one of the " +
+				"VISUAL or EDITOR environment variables.")
+		}
+		args = []string{path}
+		return args, nil
+	}
+
+	// Automatically add -w to 'code' if it is not present.
+	if args[0] == "code" && len(args) == 1 {
+		args = append(args, "-w")
 	}
 
 	path, err := edit.env.esc.exec.LookPath(args[0])
@@ -160,7 +181,38 @@ func (edit *envEditCommand) getEditor() ([]string, error) {
 	return args, nil
 }
 
-func (edit *envEditCommand) editWithYAMLEditor(editor []string, yaml, checked []byte) ([]byte, error) {
+func (edit *envEditCommand) editWithYAMLEditor(
+	editor []string,
+	envName string,
+	yaml []byte,
+	checked *esc.Environment,
+	diags []client.EnvironmentDiagnostic,
+) ([]byte, error) {
+	var details bytes.Buffer
+	if len(diags) != 0 {
+		var tmp bytes.Buffer
+		fmt.Fprintln(&tmp, "# Diagnostics")
+		fmt.Fprintln(&tmp, "")
+		err := edit.env.writeYAMLEnvironmentDiagnostics(&tmp, envName, yaml, diags)
+		contract.IgnoreError(err)
+
+		fmt.Fprintln(&details, "---")
+		fmt.Fprint(&details, strings.ReplaceAll(tmp.String(), "\n", "\n# "))
+	}
+	if checked != nil {
+		fmt.Fprintln(&details, "---")
+		fmt.Fprintln(&details, "# Please edit the environment definition above.")
+		fmt.Fprintln(&details, "# The object below is the current result of")
+		fmt.Fprintln(&details, "# evaluating the environment and will not be")
+		fmt.Fprintln(&details, "# saved. An empty definition aborts the edit.")
+		fmt.Fprintln(&details, "")
+
+		enc := json.NewEncoder(&details)
+		enc.SetIndent("", "  ")
+		err := enc.Encode(esc.NewValue(checked.Properties).ToJSON(false))
+		contract.IgnoreError(err)
+	}
+
 	filename, err := func() (string, error) {
 		filename, f, err := edit.env.esc.fs.CreateTemp("", "*.yaml")
 		if err != nil {
@@ -174,18 +226,12 @@ func (edit *envEditCommand) editWithYAMLEditor(editor []string, yaml, checked []
 			return "", err
 		}
 
-		if len(checked) != 0 {
+		if details.Len() != 0 {
 			if len(yaml) != 0 && yaml[len(yaml)-1] != '\n' {
 				fmt.Fprintln(f, "")
 			}
 
-			fmt.Fprintln(f, "---")
-			fmt.Fprintln(f, "# Please edit the environment definition above.")
-			fmt.Fprintln(f, "# The object below is the current result of")
-			fmt.Fprintln(f, "# evaluating the environment and will not be")
-			fmt.Fprintln(f, "# saved.")
-			fmt.Fprintln(f, "")
-			if _, err = f.Write(checked); err != nil {
+			if _, err = f.Write(details.Bytes()); err != nil {
 				rmErr := edit.env.esc.fs.Remove(filename)
 				contract.IgnoreError(rmErr)
 				return "", err

cmd/esc/cli/env_open.go

@@ -6,12 +6,14 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"sort"
 	"time"
 
 	"github.com/pulumi/esc"
 	"github.com/pulumi/esc/cmd/esc/cli/client"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
 	"github.com/spf13/cobra"
+	"golang.org/x/exp/maps"
 )
 
 func newEnvOpenCmd(envcmd *envCommand) *cobra.Command {
@@ -88,25 +90,13 @@ func newEnvOpenCmd(envcmd *envCommand) *cobra.Command {
 				enc.SetIndent("", "  ")
 				return enc.Encode(val)
 			case "dotenv":
-				vars, ok := env.Properties["environmentVariables"].Value.(map[string]esc.Value)
-				if !ok {
-					return nil
-				}
-				for k, v := range vars {
-					if strValue, ok := v.Value.(string); ok {
-						fmt.Fprintf(envcmd.esc.stdout, "%v=%q\n", k, strValue)
-					}
+				for _, kvp := range getEnvironmentVariables(env) {
+					fmt.Fprintln(envcmd.esc.stdout, kvp)
 				}
 				return nil
 			case "shell":
-				vars, ok := env.Properties["environmentVariables"].Value.(map[string]esc.Value)
-				if !ok {
-					return nil
-				}
-				for k, v := range vars {
-					if strValue, ok := v.Value.(string); ok {
-						fmt.Fprintf(envcmd.esc.stdout, "export %v=%q\n", k, strValue)
-					}
+				for _, kvp := range getEnvironmentVariables(env) {
+					fmt.Fprintf(envcmd.esc.stdout, "export %v\n", kvp)
 				}
 				return nil
 			case "string":
@@ -129,6 +119,24 @@ func newEnvOpenCmd(envcmd *envCommand) *cobra.Command {
 	return cmd
 }
 
+func getEnvironmentVariables(env *esc.Environment) []string {
+	vars, ok := env.Properties["environmentVariables"].Value.(map[string]esc.Value)
+	if !ok {
+		return nil
+	}
+	keys := maps.Keys(vars)
+	sort.Strings(keys)
+
+	var environ []string
+	for _, k := range keys {
+		v := vars[k]
+		if strValue, ok := v.Value.(string); ok {
+			environ = append(environ, fmt.Sprintf("%v=%q", k, strValue))
+		}
+	}
+	return environ
+}
+
 func (env *envCommand) openEnvironment(
 	ctx context.Context,
 	orgName string,

cmd/esc/cli/testdata/env-edit-editor-abort.yaml

@@ -0,0 +1,13 @@
+run: env edit test
+process:
+  environ:
+    EDITOR: my-editor
+  commands:
+    my-editor: |
+      echo -n "" >$1
+environments:
+  test-user/test:
+    values:
+      foo: bar
+stderr: |
+  Aborting edit due to empty definition.

cmd/esc/cli/testdata/env-edit-editor-invalid.yaml

@@ -0,0 +1,19 @@
+run: env edit test
+process:
+  environ:
+    EDITOR: my-editor
+  commands:
+    my-editor: |
+      echo -e "imports:\n" >$1
+environments:
+  test-user/test:
+    values:
+      foo: bar
+stderr: |
+  Error: imports must be a list
+
+    on test line 1:
+     1: imports:
+
+  Press ENTER to continue editing or ^D to exit
+  Aborting edit.

cmd/esc/cli/testdata/env-edit-editor-ok.yaml

@@ -9,3 +9,5 @@ environments:
   test-user/test:
     values:
       foo: bar
+stdout: |
+  Environment updated.

cmd/esc/cli/testdata/env-edit-flag-code-ok.yaml

@@ -0,0 +1,12 @@
+run: env edit test --editor code
+process:
+  commands:
+    code: |
+      if [[ "$1" -ne "-w" ]]; then exit 1; fi
+      echo -e "values:\n  foo: baz\n" >$2
+environments:
+  test-user/test:
+    values:
+      foo: bar
+stdout: |
+  Environment updated.

cmd/esc/cli/testdata/env-edit-flag-ok.yaml

@@ -7,3 +7,5 @@ environments:
   test-user/test:
     values:
       foo: bar
+stdout: |
+  Environment updated.

cmd/esc/cli/testdata/env-edit-none-not-found.yaml

@@ -0,0 +1,4 @@
+run: env edit test
+error: No available editor. Please use the --editor flag or set one of the VISUAL or EDITOR environment variables.
+stderr: |
+  Error: No available editor. Please use the --editor flag or set one of the VISUAL or EDITOR environment variables.

cmd/esc/cli/testdata/env-edit-none-ok.yaml

@@ -0,0 +1,11 @@
+run: env edit test
+process:
+  commands:
+    vi: |
+      echo -e "values:\n  foo: baz\n" >$1
+environments:
+  test-user/test:
+    values:
+      foo: bar
+stdout: |
+  Environment updated.

cmd/esc/cli/testdata/env-edit-visual-ok.yaml

@@ -9,3 +9,5 @@ environments:
   test-user/test:
     values:
       foo: bar
+stdout: |
+  Environment updated.