Repository: digitalocean/doctl
Pull request #1265 (2022-09-30 20:50:38)

Migrate existing confirmation dialogs to use confirm charm.
This migrates all of our existing confirmation dialogs to use the new `confirm` package added with the `app dev` command. This seems like a good place to start incorporating that work into the rest of `doctl` as we get a lot of bang for our buck. Changing the existing helper method brings it into many of our existing commands. Though for that same reason, I'm opening this as a draft for discussion first. We need to ensure there is no unexpected breakage.

The biggest functional difference is that you can now use the arrow keys to move between the options rather than just typing, but typing `y` or `n` still works. It's not a huge change, but I think it brings a more modern feeling.

Old prompt:

![image](https://user-images.githubusercontent.com/46943/193353458-7ba50fc6-8c5f-4317-a245-b7e2f23c2a3e.png)

Old no selected:

![image](https://user-images.githubusercontent.com/46943/193353550-cf085acc-ee8e-4dd2-a745-e5ce50f7f8b2.png)

Old yes selected:

![image](https://user-images.githubusercontent.com/46943/193353629-0d3cd9db-d2ed-4799-84e7-df9408a0697f.png)


New propmt: 

![image](https://user-images.githubusercontent.com/46943/193353316-b088c44c-9425-4f36-b181-8406b9731aa2.png)

New no selected: 

![image](https://user-images.githubusercontent.com/46943/193353401-df5dbfee-037b-4f75-9298-2ea5d457c451.png)

New yes selected:

![image](https://user-images.githubusercontent.com/46943/193353776-42434fb4-b0de-4ebe-9b26-420042b84ab1.png)



Patch:
diff --git a/commands/confirmation.go b/commands/confirmation.go
index be0c55aa..b0902310 100644
--- a/commands/confirmation.go
+++ b/commands/confirmation.go
@@ -1,43 +1,27 @@
 package commands
 
 import (
-	"bufio"
 	"fmt"
-	"io"
-	"os"
-	"strings"
-)
-
-// retrieveUserInput is a function that can retrieve user input in form of string. By default,
-// it will prompt the user. In test, you can replace this with code that returns the appropriate response.
-var retrieveUserInput = func(message string) (string, error) {
-	return readUserInput(os.Stdin, message)
-}
-
-// readUserInput is similar to retrieveUserInput but takes an explicit
-// io.Reader to read user input from. It is meant to allow simplified testing
-// as to-be-read inputs can be injected conveniently.
-func readUserInput(in io.Reader, message string) (string, error) {
-	reader := bufio.NewReader(in)
-	warnConfirm("Are you sure you want to " + message + " (y/N) ? ")
-	answer, err := reader.ReadString('\n')
-	if err != nil {
-		return "", err
-	}
 
-	answer = strings.TrimRight(answer, "\r\n")
-
-	return strings.ToLower(answer), nil
-}
+	"github.com/digitalocean/doctl/commands/charm/confirm"
+	"github.com/digitalocean/doctl/commands/charm/template"
+)
 
 // AskForConfirm parses and verifies user input for confirmation.
 func AskForConfirm(message string) error {
-	answer, err := retrieveUserInput(message)
+	if !Interactive {
+		warn("Requires confirmation. Use the `--force` flag to continue without confirmation.")
+		return ErrExitSilently
+	}
+	choice, err := confirm.New(
+		template.String("Are you sure you want to {{.}}", message),
+		confirm.WithDefaultChoice(confirm.No),
+	).Prompt()
 	if err != nil {
-		return fmt.Errorf("Unable to parse users input: %s", err)
+		return err
 	}
 
-	if answer != "y" && answer != "ye" && answer != "yes" {
+	if choice != confirm.Yes {
 		return fmt.Errorf("Invalid user input")
 	}
 


Test patch:
diff --git a/commands/confirmation_test.go b/commands/confirmation_test.go
deleted file mode 100644
index 926438d0..00000000
--- a/commands/confirmation_test.go
+++ /dev/null
@@ -1,90 +0,0 @@
-package commands
-
-import (
-	"fmt"
-	"strings"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-func TestAskForConfirmYes(t *testing.T) {
-	rui := retrieveUserInput
-	defer func() {
-		retrieveUserInput = rui
-	}()
-
-	retrieveUserInput = func(string) (string, error) {
-		return "yes", nil
-	}
-
-	err := AskForConfirm("test")
-	assert.NoError(t, err)
-}
-
-func TestAskForConfirmNo(t *testing.T) {
-	rui := retrieveUserInput
-	defer func() {
-		retrieveUserInput = rui
-	}()
-
-	retrieveUserInput = func(string) (string, error) {
-		return "no", nil
-	}
-
-	err := AskForConfirm("test")
-	assert.Error(t, err)
-}
-
-func TestAskForConfirmAny(t *testing.T) {
-	rui := retrieveUserInput
-	defer func() {
-		retrieveUserInput = rui
-	}()
-
-	retrieveUserInput = func(string) (string, error) {
-		return "some-random-message", nil
-	}
-
-	err := AskForConfirm("test")
-	assert.Error(t, err)
-}
-
-func TestAskForConfirmError(t *testing.T) {
-	rui := retrieveUserInput
-	defer func() {
-		retrieveUserInput = rui
-	}()
-
-	retrieveUserInput = func(string) (string, error) {
-		return "", fmt.Errorf("test-error")
-	}
-
-	err := AskForConfirm("test")
-	assert.Error(t, err)
-}
-
-func TestReadUserInput(t *testing.T) {
-	tests := []struct {
-		in   string
-		want string
-	}{
-		{
-			in:   "UNIX\n",
-			want: "unix",
-		},
-		{
-			in:   "Windows\r\n",
-			want: "windows",
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.want, func(t *testing.T) {
-			buf := strings.NewReader(tt.in)
-			answer, err := readUserInput(buf, "msg")
-			require.NoError(t, err)
-			assert.Equal(t, tt.want, answer)
-		})
-	}
-}
diff --git a/commands/namespaces_test.go b/commands/namespaces_test.go
index 62f9984b..08ef7427 100644
--- a/commands/namespaces_test.go
+++ b/commands/namespaces_test.go
@@ -244,11 +244,6 @@ func TestNamespacesDelete(t *testing.T) {
 						APIHost:   "https://sgp1.example.com",
 					},
 				}}
-				// Confirmation dialog can't be fully mocked but it can be replaced
-				// (as in confirmation_test.go).
-				retrieveUserInput = func(string) (string, error) {
-					return "no", nil
-				}
 
 				ctx := context.TODO()
 				tm.serverless.EXPECT().ListNamespaces(ctx).Return(listForMatching, nil)
diff --git a/integration/domain_records_delete_test.go b/integration/domain_records_delete_test.go
index 63d25c0f..d3889155 100644
--- a/integration/domain_records_delete_test.go
+++ b/integration/domain_records_delete_test.go
@@ -114,7 +114,7 @@ var _ = suite("compute/domain/records/delete", func(t *testing.T, when spec.G, i
 	})
 
 	when("deleting one domain record without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -128,12 +128,12 @@ var _ = suite("compute/domain/records/delete", func(t *testing.T, when spec.G, i
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(domainRecDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 
 	when("deleting two domain records without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -148,12 +148,7 @@ var _ = suite("compute/domain/records/delete", func(t *testing.T, when spec.G, i
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(multiDomainRecDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 })
-
-const (
-	domainRecDelOutput      = "Warning: Are you sure you want to delete this domain record? (y/N) ? Error: Operation aborted."
-	multiDomainRecDelOutput = "Warning: Are you sure you want to delete 2 domain records? (y/N) ? Error: Operation aborted."
-)
diff --git a/integration/droplet_delete_test.go b/integration/droplet_delete_test.go
index 6686fdd4..423974d9 100644
--- a/integration/droplet_delete_test.go
+++ b/integration/droplet_delete_test.go
@@ -136,7 +136,7 @@ var _ = suite("compute/droplet/delete", func(t *testing.T, when spec.G, it spec.
 	})
 
 	when("deleting one Droplet without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -148,30 +148,12 @@ var _ = suite("compute/droplet/delete", func(t *testing.T, when spec.G, it spec.
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(dropletDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 
-	when("deleting two Droplet without force flag", func() {
-		it("correctly prompts for confirmation", func() {
-			cmd := exec.Command(builtBinaryPath,
-				"-t", "some-magic-token",
-				"-u", server.URL,
-				"compute",
-				"droplet",
-				"delete",
-				"some-droplet-name",
-				"another-droplet-name",
-			)
-
-			output, err := cmd.CombinedOutput()
-			expect.Error(err)
-			expect.Equal(strings.TrimSpace(multiDropletDelOutput), strings.TrimSpace(string(output)))
-		})
-	})
-
-	when("deleting one Droplet by tag without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+	when("deleting Droplet by tag without force flag", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -183,12 +165,12 @@ var _ = suite("compute/droplet/delete", func(t *testing.T, when spec.G, it spec.
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(tagDropletDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 
 	when("deleting two Droplet by tag without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -200,14 +182,7 @@ var _ = suite("compute/droplet/delete", func(t *testing.T, when spec.G, it spec.
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(tagMultiDropletDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 })
-
-const (
-	dropletDelOutput         = "Warning: Are you sure you want to delete this Droplet? (y/N) ? Error: Operation aborted."
-	multiDropletDelOutput    = "Warning: Are you sure you want to delete 2 Droplets? (y/N) ? Error: Operation aborted."
-	tagDropletDelOutput      = `Warning: Are you sure you want to delete 1 Droplet tagged "one"? [affected Droplet: 1337] (y/N) ? Error: Operation aborted.`
-	tagMultiDropletDelOutput = `Warning: Are you sure you want to delete 2 Droplets tagged "two"? [affected Droplets: 1337 7331] (y/N) ? Error: Operation aborted.`
-)
diff --git a/integration/firewall_delete_test.go b/integration/firewall_delete_test.go
index bc9c7028..fc3aa3bc 100644
--- a/integration/firewall_delete_test.go
+++ b/integration/firewall_delete_test.go
@@ -98,7 +98,7 @@ var _ = suite("compute/firewall/delete", func(t *testing.T, when spec.G, it spec
 	})
 
 	when("deleting one firewall without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -110,12 +110,12 @@ var _ = suite("compute/firewall/delete", func(t *testing.T, when spec.G, it spec
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(fwDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 
 	when("deleting two firewalls without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -128,12 +128,7 @@ var _ = suite("compute/firewall/delete", func(t *testing.T, when spec.G, it spec
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(multiFwDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 })
-
-const (
-	fwDelOutput      = "Warning: Are you sure you want to delete this firewall? (y/N) ? Error: Operation aborted."
-	multiFwDelOutput = "Warning: Are you sure you want to delete 2 firewalls? (y/N) ? Error: Operation aborted."
-)
diff --git a/integration/image_delete_test.go b/integration/image_delete_test.go
index a423cf6c..f5dac847 100644
--- a/integration/image_delete_test.go
+++ b/integration/image_delete_test.go
@@ -98,7 +98,7 @@ var _ = suite("compute/image/delete", func(t *testing.T, when spec.G, it spec.S)
 	})
 
 	when("deleting one image without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -110,12 +110,12 @@ var _ = suite("compute/image/delete", func(t *testing.T, when spec.G, it spec.S)
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(imageDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 
 	when("deleting two images without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -128,12 +128,7 @@ var _ = suite("compute/image/delete", func(t *testing.T, when spec.G, it spec.S)
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(multiImageDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 })
-
-const (
-	imageDelOutput      = "Warning: Are you sure you want to delete this image? (y/N) ? Error: Operation aborted."
-	multiImageDelOutput = "Warning: Are you sure you want to delete 2 images? (y/N) ? Error: Operation aborted."
-)
diff --git a/integration/init_test.go b/integration/init_test.go
index e6d9cdbf..ab417663 100644
--- a/integration/init_test.go
+++ b/integration/init_test.go
@@ -21,6 +21,10 @@ var (
 	builtBinaryPath string
 )
 
+const (
+	confirmNonInteractiveOutput = "Warning: Requires confirmation. Use the `--force` flag to continue without confirmation.\nError: Operation aborted."
+)
+
 func TestRun(t *testing.T) {
 	suite.Run(t)
 }
diff --git a/integration/snapshot_delete_test.go b/integration/snapshot_delete_test.go
index d13ea1a3..74a4db35 100644
--- a/integration/snapshot_delete_test.go
+++ b/integration/snapshot_delete_test.go
@@ -110,7 +110,7 @@ var _ = suite("compute/snapshot/delete", func(t *testing.T, when spec.G, it spec
 	})
 
 	when("deleting one snapshot without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -122,12 +122,12 @@ var _ = suite("compute/snapshot/delete", func(t *testing.T, when spec.G, it spec
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(snapshotDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 
 	when("deleting two snapshots without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -140,12 +140,7 @@ var _ = suite("compute/snapshot/delete", func(t *testing.T, when spec.G, it spec
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(multiSnapshotDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 })
-
-const (
-	snapshotDelOutput      = "Warning: Are you sure you want to delete this snapshot? (y/N) ? Error: Operation aborted."
-	multiSnapshotDelOutput = "Warning: Are you sure you want to delete 2 snapshots? (y/N) ? Error: Operation aborted."
-)
diff --git a/integration/tags_delete_test.go b/integration/tags_delete_test.go
index a854202b..b0bb3ddc 100644
--- a/integration/tags_delete_test.go
+++ b/integration/tags_delete_test.go
@@ -110,7 +110,7 @@ var _ = suite("compute/tags/delete", func(t *testing.T, when spec.G, it spec.S)
 	})
 
 	when("deleting one tag without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -122,12 +122,12 @@ var _ = suite("compute/tags/delete", func(t *testing.T, when spec.G, it spec.S)
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(tagDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 
 	when("deleting two tags without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -140,12 +140,7 @@ var _ = suite("compute/tags/delete", func(t *testing.T, when spec.G, it spec.S)
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(multiTagDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 })
-
-const (
-	tagDelOutput      = "Warning: Are you sure you want to delete this tag? (y/N) ? Error: Operation aborted."
-	multiTagDelOutput = "Warning: Are you sure you want to delete 2 tags? (y/N) ? Error: Operation aborted."
-)