Skip to content

feat: Add confirm prompts to some cli actions #1591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
May 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions cli/cliui/prompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,21 @@ type PromptOptions struct {
Validate func(string) error
}

func AllowSkipPrompt(cmd *cobra.Command) {
cmd.Flags().BoolP("yes", "y", false, "Bypass prompts")
}

// Prompt asks the user for input.
func Prompt(cmd *cobra.Command, opts PromptOptions) (string, error) {
// If the cmd has a "yes" flag for skipping confirm prompts, honor it.
// If it's not a "Confirm" prompt, then don't skip. As the default value of
// "yes" makes no sense.
if opts.IsConfirm && cmd.Flags().Lookup("yes") != nil {
if skip, _ := cmd.Flags().GetBool("yes"); skip {
return "yes", nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we print to the terminal here e.g. Confirm create? (yes/no) yes, or Skipping: Confirm create? (yes/no) just to make it clear that the --yes flag had an effect?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mafredri I was unsure. Do other applications do this? apt-get does not have any indication for example.

For scripting purposes, I would imagine we want to reduce output to near 0 if we can.

}
}

_, _ = fmt.Fprint(cmd.OutOrStdout(), Styles.FocusedPrompt.String()+opts.Text+" ")
if opts.IsConfirm {
opts.Default = "yes"
Expand Down
62 changes: 55 additions & 7 deletions cli/cliui/prompt_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package cliui_test

import (
"bytes"
"context"
"io"
"os"
"os/exec"
"testing"
Expand All @@ -24,7 +26,7 @@ func TestPrompt(t *testing.T) {
go func() {
resp, err := newPrompt(ptty, cliui.PromptOptions{
Text: "Example",
})
}, nil)
require.NoError(t, err)
msgChan <- resp
}()
Expand All @@ -41,7 +43,7 @@ func TestPrompt(t *testing.T) {
resp, err := newPrompt(ptty, cliui.PromptOptions{
Text: "Example",
IsConfirm: true,
})
}, nil)
require.NoError(t, err)
doneChan <- resp
}()
Expand All @@ -50,14 +52,55 @@ func TestPrompt(t *testing.T) {
require.Equal(t, "yes", <-doneChan)
})

t.Run("Skip", func(t *testing.T) {
t.Parallel()
ptty := ptytest.New(t)
var buf bytes.Buffer

// Copy all data written out to a buffer. When we close the ptty, we can
// no longer read from the ptty.Output(), but we can read what was
// written to the buffer.
dataRead, doneReading := context.WithTimeout(context.Background(), time.Second*2)
go func() {
// This will throw an error sometimes. The underlying ptty
// has its own cleanup routines in t.Cleanup. Instead of
// trying to control the close perfectly, just let the ptty
// double close. This error isn't important, we just
// want to know the ptty is done sending output.
_, _ = io.Copy(&buf, ptty.Output())
doneReading()
}()

doneChan := make(chan string)
go func() {
resp, err := newPrompt(ptty, cliui.PromptOptions{
Text: "ShouldNotSeeThis",
IsConfirm: true,
}, func(cmd *cobra.Command) {
cliui.AllowSkipPrompt(cmd)
cmd.SetArgs([]string{"-y"})
})
require.NoError(t, err)
doneChan <- resp
}()

require.Equal(t, "yes", <-doneChan)
// Close the reader to end the io.Copy
require.NoError(t, ptty.Close(), "close eof reader")
// Wait for the IO copy to finish
<-dataRead.Done()
// Timeout error means the output was hanging
require.ErrorIs(t, dataRead.Err(), context.Canceled, "should be canceled")
require.Len(t, buf.Bytes(), 0, "expect no output")
})
t.Run("JSON", func(t *testing.T) {
t.Parallel()
ptty := ptytest.New(t)
doneChan := make(chan string)
go func() {
resp, err := newPrompt(ptty, cliui.PromptOptions{
Text: "Example",
})
}, nil)
require.NoError(t, err)
doneChan <- resp
}()
Expand All @@ -73,7 +116,7 @@ func TestPrompt(t *testing.T) {
go func() {
resp, err := newPrompt(ptty, cliui.PromptOptions{
Text: "Example",
})
}, nil)
require.NoError(t, err)
doneChan <- resp
}()
Expand All @@ -89,7 +132,7 @@ func TestPrompt(t *testing.T) {
go func() {
resp, err := newPrompt(ptty, cliui.PromptOptions{
Text: "Example",
})
}, nil)
require.NoError(t, err)
doneChan <- resp
}()
Expand All @@ -101,7 +144,7 @@ func TestPrompt(t *testing.T) {
})
}

func newPrompt(ptty *ptytest.PTY, opts cliui.PromptOptions) (string, error) {
func newPrompt(ptty *ptytest.PTY, opts cliui.PromptOptions, cmdOpt func(cmd *cobra.Command)) (string, error) {
value := ""
cmd := &cobra.Command{
RunE: func(cmd *cobra.Command, args []string) error {
Expand All @@ -110,7 +153,12 @@ func newPrompt(ptty *ptytest.PTY, opts cliui.PromptOptions) (string, error) {
return err
},
}
cmd.SetOutput(ptty.Output())
// Optionally modify the cmd
if cmdOpt != nil {
cmdOpt(cmd)
}
cmd.SetOut(ptty.Output())
cmd.SetErr(ptty.Output())
cmd.SetIn(ptty.Input())
return value, cmd.ExecuteContext(context.Background())
}
Expand Down
1 change: 1 addition & 0 deletions cli/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ func create() *cobra.Command {
},
}

cliui.AllowSkipPrompt(cmd)
cliflag.StringVarP(cmd.Flags(), &templateName, "template", "t", "CODER_TEMPLATE_NAME", "", "Specify a template name.")
cliflag.StringVarP(cmd.Flags(), &parameterFile, "parameter-file", "", "CODER_PARAMETER_FILE", "", "Specify a file path with parameter values.")
return cmd
Expand Down
28 changes: 10 additions & 18 deletions cli/create_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package cli_test

import (
"context"
"fmt"
"os"
"testing"
"time"

"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -46,34 +48,24 @@ func TestCreate(t *testing.T) {
<-doneChan
})

t.Run("CreateFromList", func(t *testing.T) {
t.Run("CreateFromListWithSkip", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
_ = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
cmd, root := clitest.New(t, "create", "my-workspace")
cmd, root := clitest.New(t, "create", "my-workspace", "-y")
clitest.SetupConfig(t, client, root)
doneChan := make(chan struct{})
pty := ptytest.New(t)
cmd.SetIn(pty.Input())
cmd.SetOut(pty.Output())
cmdCtx, done := context.WithTimeout(context.Background(), time.Second*3)
go func() {
defer close(doneChan)
err := cmd.Execute()
defer done()
err := cmd.ExecuteContext(cmdCtx)
require.NoError(t, err)
}()
matches := []string{
"Confirm create", "yes",
}
for i := 0; i < len(matches); i += 2 {
match := matches[i]
value := matches[i+1]
pty.ExpectMatch(match)
pty.WriteLine(value)
}
<-doneChan
// No pty interaction needed since we use the -y skip prompt flag
<-cmdCtx.Done()
require.ErrorIs(t, cmdCtx.Err(), context.Canceled)
})

t.Run("FromNothing", func(t *testing.T) {
Expand Down
12 changes: 11 additions & 1 deletion cli/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,21 @@ import (

// nolint
func delete() *cobra.Command {
return &cobra.Command{
cmd := &cobra.Command{
Annotations: workspaceCommand,
Use: "delete <workspace>",
Short: "Delete a workspace",
Aliases: []string{"rm"},
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
_, err := cliui.Prompt(cmd, cliui.PromptOptions{
Text: "Confirm delete workspace?",
IsConfirm: true,
})
if err != nil {
return err
}

client, err := createClient(cmd)
if err != nil {
return err
Expand All @@ -40,4 +48,6 @@ func delete() *cobra.Command {
return cliui.WorkspaceBuild(cmd.Context(), cmd.OutOrStdout(), client, build.ID, before)
},
}
cliui.AllowSkipPrompt(cmd)
return cmd
}
2 changes: 1 addition & 1 deletion cli/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestDelete(t *testing.T) {
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
cmd, root := clitest.New(t, "delete", workspace.Name)
cmd, root := clitest.New(t, "delete", workspace.Name, "-y")
clitest.SetupConfig(t, client, root)
doneChan := make(chan struct{})
pty := ptytest.New(t)
Expand Down
12 changes: 11 additions & 1 deletion cli/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,20 @@ import (
)

func start() *cobra.Command {
return &cobra.Command{
cmd := &cobra.Command{
Annotations: workspaceCommand,
Use: "start <workspace>",
Short: "Build a workspace with the start state",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
_, err := cliui.Prompt(cmd, cliui.PromptOptions{
Text: "Confirm start workspace?",
IsConfirm: true,
})
if err != nil {
return err
}

client, err := createClient(cmd)
if err != nil {
return err
Expand All @@ -38,4 +46,6 @@ func start() *cobra.Command {
return cliui.WorkspaceBuild(cmd.Context(), cmd.OutOrStdout(), client, build.ID, before)
},
}
cliui.AllowSkipPrompt(cmd)
return cmd
}
12 changes: 11 additions & 1 deletion cli/stop.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,20 @@ import (
)

func stop() *cobra.Command {
return &cobra.Command{
cmd := &cobra.Command{
Annotations: workspaceCommand,
Use: "stop <workspace>",
Short: "Build a workspace with the stop state",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
_, err := cliui.Prompt(cmd, cliui.PromptOptions{
Text: "Confirm stop workspace?",
IsConfirm: true,
})
if err != nil {
return err
}

client, err := createClient(cmd)
if err != nil {
return err
Expand All @@ -38,4 +46,6 @@ func stop() *cobra.Command {
return cliui.WorkspaceBuild(cmd.Context(), cmd.OutOrStdout(), client, build.ID, before)
},
}
cliui.AllowSkipPrompt(cmd)
return cmd
}
17 changes: 7 additions & 10 deletions cli/templatecreate.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

func templateCreate() *cobra.Command {
var (
yes bool
directory string
provisioner string
parameterFile string
Expand Down Expand Up @@ -85,14 +84,12 @@ func templateCreate() *cobra.Command {
return err
}

if !yes {
_, err = cliui.Prompt(cmd, cliui.PromptOptions{
Text: "Confirm create?",
IsConfirm: true,
})
if err != nil {
return err
}
_, err = cliui.Prompt(cmd, cliui.PromptOptions{
Text: "Confirm create?",
IsConfirm: true,
})
if err != nil {
return err
}

_, err = client.CreateTemplate(cmd.Context(), organization.ID, codersdk.CreateTemplateRequest{
Expand Down Expand Up @@ -123,7 +120,7 @@ func templateCreate() *cobra.Command {
if err != nil {
panic(err)
}
cmd.Flags().BoolVarP(&yes, "yes", "y", false, "Bypass prompts")
cliui.AllowSkipPrompt(cmd)
return cmd
}

Expand Down
1 change: 1 addition & 0 deletions cli/templateupdate.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ func templateUpdate() *cobra.Command {
currentDirectory, _ := os.Getwd()
cmd.Flags().StringVarP(&directory, "directory", "d", currentDirectory, "Specify the directory to create from")
cmd.Flags().StringVarP(&provisioner, "test.provisioner", "", "terraform", "Customize the provisioner backend")
cliui.AllowSkipPrompt(cmd)
// This is for testing!
err := cmd.Flags().MarkHidden("test.provisioner")
if err != nil {
Expand Down