diff --git a/internal/app/action_values.go b/internal/app/action_values.go index e80a7119..54fcb54e 100644 --- a/internal/app/action_values.go +++ b/internal/app/action_values.go @@ -293,7 +293,7 @@ func (a *application) actionValue(cmd *cobra.Command, secret bool, name, body, b if secret { return a.deps.Prompter.SecretInput(cmd.Context(), "Paste the value of "+strings.ToUpper(name)+":") } - return a.deps.Prompter.Input(cmd.Context(), "Value of "+strings.ToUpper(name)+":") + return a.deps.Prompter.Ask(cmd.Context(), "Value of "+strings.ToUpper(name)+":", nil) } // envEntry is one NAME=value line of a dotenv file. @@ -348,10 +348,7 @@ $ tea %[1]s delete DEPLOY_TOKEN --org acme --yes`, kind), if err != nil { return err } - if err := a.confirmAction(cmd, terminal.Confirmation{ - Prompt: fmt.Sprintf("Delete %s %s from %s?", kind, name, selected), - NonTTY: fmt.Sprintf("delete %s %s from %s", kind, name, selected), - }); err != nil { + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: fmt.Sprintf("Delete %s %s from %s?", kind, name, selected)}); err != nil { return err } if secret { diff --git a/internal/app/actions.go b/internal/app/actions.go index a1abb9a7..b94a795e 100644 --- a/internal/app/actions.go +++ b/internal/app/actions.go @@ -399,7 +399,6 @@ func (a *application) askInput(cmd *cobra.Command, input services.WorkflowInput) if input.Description != "" { title += " — " + input.Description } - asker, rich := a.deps.Prompter.(terminal.Asker) switch chooser, ok := a.deps.Prompter.(terminal.Chooser); { case input.Type == "choice" && ok && len(input.Options) > 0: choices := make([]terminal.Choice, len(input.Options)) @@ -407,8 +406,8 @@ func (a *application) askInput(cmd *cobra.Command, input services.WorkflowInput) choices[i] = terminal.Choice{Label: option, Value: option} } return chooser.Choose(cmd.Context(), title, choices, input.Default) - case input.Type == "boolean" && rich: - yes, err := asker.ConfirmDefault(cmd.Context(), title, input.Default == "true") + case input.Type == "boolean": + yes, err := a.deps.Prompter.ConfirmDefault(cmd.Context(), title, input.Default == "true") return strconv.FormatBool(yes), err } if input.Default != "" { @@ -424,13 +423,7 @@ func (a *application) askInput(cmd *cobra.Command, input services.WorkflowInput) _, err := inputValue(input, answer) return err } - var answer string - var err error - if rich { - answer, err = asker.Ask(cmd.Context(), title, validate) - } else if answer, err = a.deps.Prompter.Input(cmd.Context(), title); err == nil { - err = validate(answer) - } + answer, err := a.deps.Prompter.Ask(cmd.Context(), title, validate) if err != nil { return "", err } diff --git a/internal/app/actions_test.go b/internal/app/actions_test.go index ea10e1db..86eed165 100644 --- a/internal/app/actions_test.go +++ b/internal/app/actions_test.go @@ -11,7 +11,6 @@ import ( "testing" "gitea.dev/tea/internal/clierr" - "gitea.dev/tea/internal/terminal" "github.com/stretchr/testify/require" ) @@ -240,5 +239,3 @@ func TestWorkflowRunFollowStreamsTheLogsOfItsJobs(t *testing.T) { require.Equal(t, clierr.Usage, clierr.ExitCode(err)) require.ErrorContains(t, err, "--follow cannot be combined with --json") } - -var _ terminal.Asker = (*dispatchingPrompter)(nil) diff --git a/internal/app/admin_users.go b/internal/app/admin_users.go index 8ad258f2..80df1408 100644 --- a/internal/app/admin_users.go +++ b/internal/app/admin_users.go @@ -354,7 +354,7 @@ $ tea admin user delete spammer --purge`, if err != nil { return err } - confirmation := terminal.Confirmation{Prompt: fmt.Sprintf("Delete user %s?", user.Username), NonTTY: "delete user " + user.Username} + confirmation := terminal.Confirmation{Prompt: "Delete user " + user.Username + "?"} if purge { confirmation = terminal.Confirmation{ Prompt: fmt.Sprintf("Delete user %s with their repositories, packages and organization memberships? This cannot be undone.", user.Username), diff --git a/internal/app/auth.go b/internal/app/auth.go index d26ce5a8..e1277729 100644 --- a/internal/app/auth.go +++ b/internal/app/auth.go @@ -758,7 +758,7 @@ instance.`, case !exists: return clierr.Wrap(clierr.Usage, t.accountError(t.User)) } - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: fmt.Sprintf("Log out %s from %s?", t.User, t.Host), NonTTY: "log out " + t.User + " from " + t.Host}); err != nil { + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: fmt.Sprintf("Log out %s from %s?", t.User, t.Host)}); err != nil { return err } manager, err := auth.NewManager(a.deps.Settings, a.deps.Credentials) diff --git a/internal/app/branches.go b/internal/app/branches.go index f2621da3..252785be 100644 --- a/internal/app/branches.go +++ b/internal/app/branches.go @@ -250,7 +250,7 @@ $ tea branch delete old-a old-b --yes`, return err } subject := branchSubject(args) - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + subject + " from " + target.Repo + "?", NonTTY: "delete " + subject + " from " + target.Repo}); err != nil { + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + subject + " from " + target.Repo + "?"}); err != nil { return err } service := services.Branches{Client: target.client} @@ -384,7 +384,7 @@ func (a *application) branchUnprotectCommand() *cobra.Command { return err } subject := "the protection of " + strings.Join(args, ", ") - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Remove " + subject + " in " + target.Repo + "?", NonTTY: "remove " + subject + " in " + target.Repo}); err != nil { + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Remove " + subject + " in " + target.Repo + "?"}); err != nil { return err } service := services.Branches{Client: target.client} diff --git a/internal/app/comments.go b/internal/app/comments.go index a0e87977..275e3c32 100644 --- a/internal/app/comments.go +++ b/internal/app/comments.go @@ -119,11 +119,7 @@ func (a *application) promptComment(cmd *cobra.Command, initial string) (string, draft, err := a.editDraft(cmd, initial) return draft.Body, draft, err } - if asker, ok := a.deps.Prompter.(terminal.Asker); ok { - body, err := asker.AskText(cmd.Context(), commentTitle, initial) - return body, terminal.Draft{}, err - } - body, err := a.deps.Prompter.Input(cmd.Context(), commentTitle) + body, err := a.deps.Prompter.AskText(cmd.Context(), commentTitle, initial) return body, terminal.Draft{}, err } @@ -189,14 +185,9 @@ func (a *application) editComment(cmd *cobra.Command, t target, comment services // deleteComment deletes comment after a confirmation that defaults to no. func (a *application) deleteComment(cmd *cobra.Command, t target, comment services.Comment) error { name := commentName(t, comment) - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + name + "?", NonTTY: "delete " + name}); err != nil { - return err - } - if err := (services.Comments{Client: t.client, Repository: t.Repo}).Delete(cmd.Context(), comment.ID); err != nil { - return err - } - a.done(cmd, "Deleted %s", name) - return nil + return a.deleteConfirmed(cmd, name, func() error { + return (services.Comments{Client: t.client, Repository: t.Repo}).Delete(cmd.Context(), comment.ID) + }) } // commentView shows one comment; now dates it on a terminal. diff --git a/internal/app/editor.go b/internal/app/editor.go index 68a6d559..884576a5 100644 --- a/internal/app/editor.go +++ b/internal/app/editor.go @@ -54,9 +54,9 @@ func (a *application) editDraft(cmd *cobra.Command, body string) (terminal.Draft // already given is never lost to a missing editor. func (a *application) bodyEditor(cmd *cobra.Command) terminal.BodyEditor { preferences, err := a.preferences() - if asker, ok := a.deps.Prompter.(terminal.Asker); ok && err == nil && preferences.Editor == "" { + if err == nil && preferences.Editor == "" { return func(ctx context.Context, initial string) (terminal.Draft, error) { - body, err := asker.AskText(ctx, terminal.BodyTitle, initial) + body, err := a.deps.Prompter.AskText(ctx, terminal.BodyTitle, initial) return terminal.Draft{Body: body}, err } } diff --git a/internal/app/editor_test.go b/internal/app/editor_test.go index d5eb14cb..3d423ec3 100644 --- a/internal/app/editor_test.go +++ b/internal/app/editor_test.go @@ -323,7 +323,7 @@ func TestReviewLifecycleEditorRejectsEmptyRequiredText(t *testing.T) { func TestBodyEditorTypesInlineWithoutAnEditor(t *testing.T) { deps, _, _ := testDeps(t) - prompt := &askingPrompter{} + prompt := &scriptedPrompter{} deps.Prompter = prompt a, cmd := outputApplication(deps, "") draft, err := a.bodyEditor(cmd)(context.Background(), "## Steps") @@ -340,10 +340,4 @@ func TestBodyEditorTypesInlineWithoutAnEditor(t *testing.T) { require.Equal(t, terminal.Draft{Path: path, Body: "edited body"}, draft) require.Equal(t, "## Steps", editor.initial) require.Len(t, prompt.texts, 1) - - deps.Prompter = &scriptedPrompter{} - deps.Getenv = envFunc(nil) - a, cmd = outputApplication(deps, "") - _, err = a.bodyEditor(cmd)(context.Background(), "") - require.ErrorContains(t, err, "configure an editor") } diff --git a/internal/app/helpers_test.go b/internal/app/helpers_test.go index 3e3ab191..e7f05111 100644 --- a/internal/app/helpers_test.go +++ b/internal/app/helpers_test.go @@ -5,6 +5,7 @@ package app import ( "bytes" + "cmp" "context" "encoding/json" "errors" @@ -153,17 +154,34 @@ type scriptedPrompter struct { asked []string secrets []string confirmations []string + texts []string + defaults []bool } -func (p *scriptedPrompter) Input(_ context.Context, field string) (string, error) { +func (p *scriptedPrompter) Ask(_ context.Context, field string, validate func(string) error) (string, error) { p.asked = append(p.asked, field) - if value, ok := p.inputs[field]; ok { + value, ok := p.inputs[field] + if !ok { + if p.input == "" { + return "", errors.New("unexpected input prompt " + field) + } + value = p.input + } + // A form re-prompts until the answer is valid; the script has no other answer, so it gives up. + if validate != nil && validate(value) != nil { + return "", terminal.ErrCanceled + } + return value, nil +} + +// AskText records the title and initial text, and answers like Ask, else with "typed body". +func (p *scriptedPrompter) AskText(_ context.Context, title, initial string) (string, error) { + p.asked = append(p.asked, title) + p.texts = append(p.texts, title+" "+initial) + if value, ok := p.inputs[title]; ok { return value, nil } - if p.input == "" { - return "", errors.New("unexpected input prompt " + field) - } - return p.input, nil + return cmp.Or(p.input, "typed body"), nil } func (p *scriptedPrompter) SecretInput(_ context.Context, title string) (string, error) { @@ -174,8 +192,9 @@ func (p *scriptedPrompter) SecretInput(_ context.Context, title string) (string, return p.secret, nil } -func (p *scriptedPrompter) Confirm(_ context.Context, description string) (bool, error) { +func (p *scriptedPrompter) ConfirmDefault(_ context.Context, description string, defaultYes bool) (bool, error) { p.confirmations = append(p.confirmations, description) + p.defaults = append(p.defaults, defaultYes) return p.accept, nil } diff --git a/internal/app/issue_attachments.go b/internal/app/issue_attachments.go index 3ad176b5..dd9281f4 100644 --- a/internal/app/issue_attachments.go +++ b/internal/app/issue_attachments.go @@ -145,14 +145,7 @@ $ tea %[1]s attachment upload %[2]s screenshot.png --name before.png`, words.gro return err } name := attachmentName(t, n, item) - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + name + "?", NonTTY: "delete " + name}); err != nil { - return err - } - if err := service.DeleteAttachment(cmd.Context(), n, id); err != nil { - return err - } - a.done(cmd, "Deleted %s", name) - return nil + return a.deleteConfirmed(cmd, name, func() error { return service.DeleteAttachment(cmd.Context(), n, id) }) }} var destination downloadFlags diff --git a/internal/app/label_options_test.go b/internal/app/label_options_test.go index 1b120608..4f896302 100644 --- a/internal/app/label_options_test.go +++ b/internal/app/label_options_test.go @@ -177,7 +177,7 @@ func TestLabelCreatePicksAColorAndPrompts(t *testing.T) { deps, _, _ := serverDeps(t, server.URL) require.NoError(t, Execute(context.Background(), deps, []string{"label", "create", "triage", "-R", "alice/project"})) require.Contains(t, labelColors, created[0]["color"]) - prompt := &askingPrompter{scriptedPrompter: scriptedPrompter{inputs: map[string]string{"Label name": "prompted", "Color (hex, e.g. d73a4a; empty = random):": "#00AA00", "Description (optional):": "From a prompt"}}} + prompt := &scriptedPrompter{inputs: map[string]string{"Label name": "prompted", "Color (hex, e.g. d73a4a; empty = random):": "#00AA00", "Description (optional):": "From a prompt"}} deps.Prompter = prompt deps.Streams.InTTY, deps.Streams.ErrTTY = true, true require.NoError(t, Execute(context.Background(), deps, []string{"label", "create", "-R", "alice/project"})) diff --git a/internal/app/labels.go b/internal/app/labels.go index 769d3685..b898d9d3 100644 --- a/internal/app/labels.go +++ b/internal/app/labels.go @@ -196,12 +196,8 @@ $ tea label create bug -c ee0701 --force`, Args: usageArgs(cobra.MaximumNArgs(1) // askLabelDetails asks for the optional color and description of a label whose name was prompted. func (a *application) askLabelDetails(cmd *cobra.Command, opts *services.LabelOptions) error { - asker, ok := a.deps.Prompter.(terminal.Asker) - if !ok { - return nil - } if opts.Color == nil { - color, err := asker.Ask(cmd.Context(), "Color (hex, e.g. d73a4a; empty = random):", func(s string) error { + color, err := a.deps.Prompter.Ask(cmd.Context(), "Color (hex, e.g. d73a4a; empty = random):", func(s string) error { if s != "" && !labelColor.MatchString(strings.TrimPrefix(s, "#")) { return errors.New("use six hexadecimal digits") } @@ -215,7 +211,7 @@ func (a *application) askLabelDetails(cmd *cobra.Command, opts *services.LabelOp } } if opts.Description == nil { - description, err := asker.Ask(cmd.Context(), "Description (optional):", nil) + description, err := a.deps.Prompter.Ask(cmd.Context(), "Description (optional):", nil) if err != nil { return err } @@ -288,7 +284,7 @@ $ tea label delete wontfix duplicate --yes`, Annotations: map[string]string{"tea if len(found) > 1 { subject = fmt.Sprintf("%d labels (%s)", len(found), strings.Join(names, ", ")) } - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + subject + " from " + s.Scope() + "?", NonTTY: "delete " + subject + " from " + s.Scope()}); err != nil { + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + subject + " from " + s.Scope() + "?"}); err != nil { return err } return batchDone(a, cmd, found, func(item services.Label) string { return "label " + strconv.Quote(item.Name) }, func(item services.Label) (services.Label, error) { diff --git a/internal/app/metadata.go b/internal/app/metadata.go index d267d66e..d457e2b0 100644 --- a/internal/app/metadata.go +++ b/internal/app/metadata.go @@ -357,7 +357,7 @@ $ tea milestone delete 7 --yes`, Annotations: map[string]string{"tea.confirm": " return err } name := fmt.Sprintf("milestone %q (ID %d) from %s", item.Title, item.ID, t.Repo) - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + name + "?", NonTTY: "delete " + name}); err != nil { + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + name + "?"}); err != nil { return err } if err := s.Delete(cmd.Context(), item.ID); err != nil { diff --git a/internal/app/prompts.go b/internal/app/prompts.go index 79fd9dc8..c8a9a034 100644 --- a/internal/app/prompts.go +++ b/internal/app/prompts.go @@ -19,6 +19,19 @@ func (a *application) confirmAction(cmd *cobra.Command, c terminal.Confirmation) return terminal.ConfirmAction(cmd.Context(), c, a.options.yes, a.deps.Streams, a.policy(s), a.deps.Prompter) } +// deleteConfirmed asks to delete subject, such as `label "bug" from alice/x`, deletes it with del +// and reports "Deleted ". +func (a *application) deleteConfirmed(cmd *cobra.Command, subject string, del func() error) error { + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + subject + "?"}); err != nil { + return err + } + if err := del(); err != nil { + return err + } + a.done(cmd, "Deleted %s", subject) + return nil +} + // requireInput fills a missing value from a prompt titled label, such as "Label name", re-prompting // until the answer is non-empty and validate, when set, accepts it. Without prompts it returns a // usage error naming --flag. It reports whether it prompted. diff --git a/internal/app/prompts_test.go b/internal/app/prompts_test.go index 6f992626..646c93c9 100644 --- a/internal/app/prompts_test.go +++ b/internal/app/prompts_test.go @@ -4,7 +4,6 @@ package app import ( - "context" "testing" "gitea.dev/tea/internal/clierr" @@ -12,27 +11,6 @@ import ( "github.com/stretchr/testify/require" ) -// askingPrompter is scriptedPrompter with the terminal.Asker extension that FormPrompter has. -type askingPrompter struct { - scriptedPrompter - texts []string - defaults []bool -} - -func (p *askingPrompter) Ask(ctx context.Context, title string, _ func(string) error) (string, error) { - return p.Input(ctx, title) -} - -func (p *askingPrompter) AskText(_ context.Context, title, initial string) (string, error) { - p.texts = append(p.texts, title+" "+initial) - return "typed body", nil -} - -func (p *askingPrompter) ConfirmDefault(ctx context.Context, title string, defaultYes bool) (bool, error) { - p.defaults = append(p.defaults, defaultYes) - return p.Confirm(ctx, title) -} - func TestConfirmActionFollowsThePromptPolicy(t *testing.T) { c := terminal.Confirmation{Prompt: `Delete label "bug" from alice/x?`, NonTTY: `delete label "bug" from alice/x`} deps, _, _ := testDeps(t) @@ -43,7 +21,7 @@ func TestConfirmActionFollowsThePromptPolicy(t *testing.T) { a.options.yes = true require.NoError(t, a.confirmAction(cmd, c)) - prompt := &askingPrompter{} + prompt := &scriptedPrompter{} deps.Prompter = prompt deps.Streams.InTTY, deps.Streams.ErrTTY = true, true a, cmd = outputApplication(deps, "") diff --git a/internal/app/pull_merge_form_test.go b/internal/app/pull_merge_form_test.go index 8530d7df..28c3a6cf 100644 --- a/internal/app/pull_merge_form_test.go +++ b/internal/app/pull_merge_form_test.go @@ -78,7 +78,7 @@ func TestPullMergeAsksForTheMethodAndBranchDeletion(t *testing.T) { form := &mergeForm{options: func(choices terminal.MergeFormChoices) terminal.MergeFormValues { return terminal.MergeFormValues{Method: "rebase", DeleteBranch: false} }} - prompt := &askingPrompter{} + prompt := &scriptedPrompter{} deps, _, diagnostics := serverDeps(t, server.URL) deps.MergeForm, deps.Prompter = form, prompt deps.Streams.InTTY, deps.Streams.ErrTTY, deps.Streams.OutTTY = true, true, true @@ -187,7 +187,7 @@ func TestPullMergePicksAPullRequestForTheBranch(t *testing.T) { } deps, _, _ := serverDeps(t, server.URL) deps.Getenv = serverEnv(server.URL, map[string]string{"TEA_REPO": "alice/project"}) - deps.MergeForm, deps.Prompter = form, &askingPrompter{} + deps.MergeForm, deps.Prompter = form, &scriptedPrompter{} deps.Streams.InTTY, deps.Streams.ErrTTY = true, true deps.Git = &fakeGit{results: map[string]string{"symbolic-ref --short HEAD": "main"}} require.NoError(t, Execute(context.Background(), deps, []string{"pr", "merge"})) diff --git a/internal/app/pull_review_form_test.go b/internal/app/pull_review_form_test.go index d1341ea9..509155fb 100644 --- a/internal/app/pull_review_form_test.go +++ b/internal/app/pull_review_form_test.go @@ -37,7 +37,7 @@ func TestPullReviewImplicitInteractiveForm(t *testing.T) { })) defer server.Close() calls := 0 - prompt := &askingPrompter{scriptedPrompter: scriptedPrompter{accept: true}} + prompt := &scriptedPrompter{accept: true} deps, out, diagnostics := serverDeps(t, server.URL) deps.Getenv = serverEnv(server.URL, map[string]string{"EDITOR": "vi"}) deps.ReviewForm = reviewFormFunc(func(_ context.Context, choices terminal.ReviewFormChoices) (terminal.ReviewFormValues, error) { @@ -78,7 +78,7 @@ func TestPullReviewFormKeepsTheBodyWhenSubmissionIsDeclined(t *testing.T) { require.Nil(t, choices.ReviewDiff, "annotating the diff needs an editor") return terminal.ReviewFormValues{Event: "COMMENT", Body: "Self note"}, nil }) - deps.Prompter = &askingPrompter{} + deps.Prompter = &scriptedPrompter{} deps.Streams.InTTY, deps.Streams.ErrTTY = true, true err := Execute(context.Background(), deps, []string{"pr", "review", "3", "-R", "alice/project"}) require.ErrorIs(t, err, terminal.ErrCanceled) diff --git a/internal/app/pull_review_lifecycle.go b/internal/app/pull_review_lifecycle.go index 2e12186f..3ec5fa91 100644 --- a/internal/app/pull_review_lifecycle.go +++ b/internal/app/pull_review_lifecycle.go @@ -134,14 +134,7 @@ func (a *application) pullReviewDeleteCommand() *cobra.Command { return err } subject := fmt.Sprintf("review %d by %s on %s#%d", id, item.Author, t.Repo, n) - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + subject + "?", NonTTY: "delete " + subject}); err != nil { - return err - } - if err := service.Delete(cmd.Context(), n, id); err != nil { - return err - } - a.done(cmd, "Deleted %s", subject) - return nil + return a.deleteConfirmed(cmd, subject, func() error { return service.Delete(cmd.Context(), n, id) }) }} cmd.Annotations["tea.confirm"] = "true" cmd.Example = "$ tea pr review delete 13 21" @@ -191,7 +184,7 @@ func (a *application) pullReviewDismissCommand(dismiss bool) *cobra.Command { } subject := fmt.Sprintf("%s on %s#%d", reviewPossessive(current), t.Repo, n) if dismiss { - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Dismiss " + subject + "?", NonTTY: "dismiss " + subject}); err != nil { + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Dismiss " + subject + "?"}); err != nil { return err } } diff --git a/internal/app/pull_review_test.go b/internal/app/pull_review_test.go index 8e1daca6..6eda1903 100644 --- a/internal/app/pull_review_test.go +++ b/internal/app/pull_review_test.go @@ -61,7 +61,7 @@ func TestReviewPromptAndFlagsReachIdenticalOptions(t *testing.T) { })) defer server.Close() deps, out, diagnostics := serverDeps(t, server.URL) - prompt := &askingPrompter{} + prompt := &scriptedPrompter{} deps.Prompter = prompt deps.Streams.InTTY, deps.Streams.ErrTTY = true, true args := []string{"pr", "review", "1", "-R", "owner/repo", "--event", "REQUEST_CHANGES"} diff --git a/internal/app/releases.go b/internal/app/releases.go index 3226fa46..2a7817ca 100644 --- a/internal/app/releases.go +++ b/internal/app/releases.go @@ -430,7 +430,7 @@ $ tea release delete v1.0.0-rc1 v1.0.0-rc2 --delete-tag --yes`, subject = "release " + args[0] + " and its tag" } } - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + subject + " from " + target.Repo + "?", NonTTY: "delete " + subject + " from " + target.Repo}); err != nil { + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + subject + " from " + target.Repo + "?"}); err != nil { return err } service := services.Releases{Client: target.client, Repository: target.Repo} diff --git a/internal/app/repo_mutations.go b/internal/app/repo_mutations.go index abd9a913..48eae56e 100644 --- a/internal/app/repo_mutations.go +++ b/internal/app/repo_mutations.go @@ -167,7 +167,7 @@ $ tea repo edit --default-merge-style squash --add-topic cli --add-topic go`, } } if options.Archived != nil && *options.Archived { - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Archive " + t.Repo + "? It becomes read-only.", NonTTY: "archive " + t.Repo}); err != nil { + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Archive " + t.Repo + "? It becomes read-only."}); err != nil { return err } } diff --git a/internal/app/ssh_keys.go b/internal/app/ssh_keys.go index e87d01df..e39cca5d 100644 --- a/internal/app/ssh_keys.go +++ b/internal/app/ssh_keys.go @@ -223,14 +223,7 @@ $ tea ssh-key delete 2 --yes`, if err != nil { return err } - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + sshKeyLabel(key) + "?", NonTTY: "delete " + sshKeyLabel(key)}); err != nil { - return err - } - if err := service.Delete(cmd.Context(), id); err != nil { - return err - } - a.done(cmd, "Deleted %s", sshKeyLabel(key)) - return nil + return a.deleteConfirmed(cmd, sshKeyLabel(key), func() error { return service.Delete(cmd.Context(), id) }) }, } } diff --git a/internal/app/target.go b/internal/app/target.go index e6ae7098..b04337d8 100644 --- a/internal/app/target.go +++ b/internal/app/target.go @@ -356,7 +356,7 @@ func (a *application) selectAccount(ctx context.Context, t *target, s settings.S if !authenticate || a.deps.Getenv("GITEA_TOKEN") != "" || !a.policy(s).CanPrompt(a.deps.Streams) || a.deps.Prompter == nil { return nil } - selected, err := a.deps.Prompter.Input(ctx, "account ("+accountNames(users)+")") + selected, err := a.deps.Prompter.Ask(ctx, "account ("+accountNames(users)+")", nil) if err != nil { return err } diff --git a/internal/app/teams.go b/internal/app/teams.go index 995d8c05..6bfd7e3d 100644 --- a/internal/app/teams.go +++ b/internal/app/teams.go @@ -459,12 +459,12 @@ $ tea team delete 7 --yes`, if team.Organization != "" { subject += " from " + team.Organization } - confirmation := terminal.Confirmation{Prompt: "Delete " + subject + "?", NonTTY: "delete " + subject} + confirmation := terminal.Confirmation{Prompt: "Delete " + subject + "?"} if team.Permission == "owner" { if !force { return clierr.Wrap(clierr.Usage, fmt.Errorf("%s is the organization's owner team and its members would lose owner access; pass --force to delete it", subject)) } - confirmation = terminal.Confirmation{Prompt: "Delete owner " + subject + "? Its members lose owner access.", Typed: team.Slug(), NonTTY: "delete owner " + subject} + confirmation = terminal.Confirmation{Prompt: "Delete owner " + subject + "? Its members lose owner access.", Typed: team.Slug()} } if err := a.confirmAction(cmd, confirmation); err != nil { return err @@ -556,10 +556,7 @@ $ tea team member add 2 bob carol`, return err } members := args[1:] - if err := a.confirmAction(cmd, terminal.Confirmation{ - Prompt: fmt.Sprintf("Remove %s from team %s?", joinNames(members), team.Slug()), - NonTTY: fmt.Sprintf("remove %s from team %s", joinNames(members), team.Slug()), - }); err != nil { + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: fmt.Sprintf("Remove %s from team %s?", joinNames(members), team.Slug())}); err != nil { return err } for _, username := range members { @@ -661,10 +658,7 @@ repositories of other owners cannot be granted to a team.`, repos = append(repos, repo) } if !grant { - if err := a.confirmAction(cmd, terminal.Confirmation{ - Prompt: fmt.Sprintf("Revoke team %s access to %s?", team.Slug(), joinNames(repos)), - NonTTY: fmt.Sprintf("revoke team %s access to %s", team.Slug(), joinNames(repos)), - }); err != nil { + if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: fmt.Sprintf("Revoke team %s access to %s?", team.Slug(), joinNames(repos))}); err != nil { return err } } diff --git a/internal/app/time.go b/internal/app/time.go index 6dbf8d6c..eab87222 100644 --- a/internal/app/time.go +++ b/internal/app/time.go @@ -196,14 +196,7 @@ $ tea time delete 12 8`, Annotations: map[string]string{"tea.text": "time delete return err } subject := fmt.Sprintf("%s logged by %s on %s#%d", spent(record.Seconds), record.User, target.Repo, n) - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + subject + "?", NonTTY: "delete " + subject}); err != nil { - return err - } - if err := service.Delete(cmd.Context(), n, id); err != nil { - return err - } - a.done(cmd, "Deleted %s", subject) - return nil + return a.deleteConfirmed(cmd, subject, func() error { return service.Delete(cmd.Context(), n, id) }) }} } @@ -214,13 +207,8 @@ func (a *application) timeResetCommand() *cobra.Command { return err } subject := fmt.Sprintf("all time logged on %s#%d", target.Repo, n) - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + subject + "?", NonTTY: "delete " + subject}); err != nil { - return err - } - if err := (services.Times{Client: target.client, Repository: target.Repo}).Reset(cmd.Context(), n); err != nil { - return err - } - a.done(cmd, "Deleted %s", subject) - return nil + return a.deleteConfirmed(cmd, subject, func() error { + return (services.Times{Client: target.client, Repository: target.Repo}).Reset(cmd.Context(), n) + }) }} } diff --git a/internal/app/webhooks.go b/internal/app/webhooks.go index 19f41ef5..6507c050 100644 --- a/internal/app/webhooks.go +++ b/internal/app/webhooks.go @@ -455,18 +455,7 @@ $ tea webhook delete 4 --org acme --yes`, if err != nil { return err } - where := webhookScopeName(service.Scope) - if err := a.confirmAction(cmd, terminal.Confirmation{ - Prompt: fmt.Sprintf("Delete %s from %s?", webhookLabel(hook), where), - NonTTY: fmt.Sprintf("delete %s from %s", webhookLabel(hook), where), - }); err != nil { - return err - } - if err := service.Delete(cmd.Context(), id); err != nil { - return err - } - a.done(cmd, "Deleted %s from %s", webhookLabel(hook), where) - return nil + return a.deleteConfirmed(cmd, webhookLabel(hook)+" from "+webhookScopeName(service.Scope), func() error { return service.Delete(cmd.Context(), id) }) }, } } diff --git a/internal/app/wiki.go b/internal/app/wiki.go index 1748672b..e45719a4 100644 --- a/internal/app/wiki.go +++ b/internal/app/wiki.go @@ -316,14 +316,9 @@ func (a *application) wikiDeleteCommand() *cobra.Command { return err } subject := "wiki page " + strconv.Quote(args[0]) + " from " + target.Repo - if err := a.confirmAction(cmd, terminal.Confirmation{Prompt: "Delete " + subject + "?", NonTTY: "delete " + subject}); err != nil { - return err - } - if err := (services.Wiki{Client: target.client, Repository: target.Repo}).Delete(cmd.Context(), args[0]); err != nil { - return err - } - a.done(cmd, "Deleted %s", subject) - return nil + return a.deleteConfirmed(cmd, subject, func() error { + return (services.Wiki{Client: target.client, Repository: target.Repo}).Delete(cmd.Context(), args[0]) + }) }, } command.ValidArgsFunction = a.completeWikiPages() diff --git a/internal/terminal/streams.go b/internal/terminal/streams.go index 767f3c14..a43bd4e8 100644 --- a/internal/terminal/streams.go +++ b/internal/terminal/streams.go @@ -72,19 +72,12 @@ var ErrCanceled = clierr.Wrap(clierr.Canceled, errors.New("canceled")) // Prompter abstracts cancellable input collection and confirmation. type Prompter interface { - Input(context.Context, string) (string, error) - SecretInput(context.Context, string) (string, error) - Confirm(context.Context, string) (bool, error) -} - -// Asker is the richer prompt API behind ConfirmAction and RequireInput: inline validation, -// multi-line text and confirmations with a default answer. FormPrompter implements it; a -// Prompter without it gets plain Input and Confirm behavior. -type Asker interface { - // Ask collects one line, re-prompting inline until validate accepts it. + // Ask collects one line, re-prompting inline until validate, when set, accepts it. Ask(ctx context.Context, title string, validate func(string) error) (string, error) // AskText collects multi-line markdown, starting from initial. AskText(ctx context.Context, title, initial string) (string, error) + // SecretInput collects a value without displaying it. + SecretInput(ctx context.Context, title string) (string, error) // ConfirmDefault asks a yes/no question that Enter answers with defaultYes. ConfirmDefault(ctx context.Context, title string, defaultYes bool) (bool, error) } @@ -124,11 +117,6 @@ func (p *FormPrompter) runForm(ctx context.Context, form *huh.Form) error { return promptError(ctx, form.WithInput(p.Streams.In).WithOutput(p.Streams.ErrOut).RunWithContext(ctx)) } -// Input collects one field using the configured terminal streams. -func (p *FormPrompter) Input(ctx context.Context, title string) (string, error) { - return p.Ask(ctx, title, nil) -} - // Ask collects one line, re-prompting inline until validate accepts the trimmed answer. func (p *FormPrompter) Ask(ctx context.Context, title string, validate func(string) error) (string, error) { var value string @@ -154,11 +142,6 @@ func (p *FormPrompter) SecretInput(ctx context.Context, title string) (string, e return value, err } -// Confirm asks a yes/no question that Enter answers with no. -func (p *FormPrompter) Confirm(ctx context.Context, title string) (bool, error) { - return p.ConfirmDefault(ctx, title, false) -} - // ConfirmDefault asks a yes/no question that Enter answers with defaultYes. func (p *FormPrompter) ConfirmDefault(ctx context.Context, title string, defaultYes bool) (bool, error) { accepted := defaultYes @@ -176,28 +159,15 @@ func RequireInput(ctx context.Context, value *string, flag, label string, valida if !policy.CanPrompt(streams) { return false, clierr.Wrap(clierr.Usage, fmt.Errorf("missing required input: supply --%s", flag)) } - var entered string - var err error - if asker, ok := prompt.(Asker); ok { - entered, err = asker.Ask(ctx, label, func(s string) error { - if s == "" { - return errors.New("a value is required") - } - if validate != nil { - return validate(s) - } - return nil - }) - } else if entered, err = prompt.Input(ctx, label); err == nil { - switch { - case entered == "": - err = clierr.Wrap(clierr.Usage, fmt.Errorf("--%s cannot be empty", flag)) - case validate != nil: - if invalid := validate(entered); invalid != nil { - err = clierr.Wrap(clierr.Usage, fmt.Errorf("--%s: %w", flag, invalid)) - } + entered, err := prompt.Ask(ctx, label, func(s string) error { + if s == "" { + return errors.New("a value is required") } - } + if validate != nil { + return validate(s) + } + return nil + }) if err != nil { return false, err } @@ -215,7 +185,8 @@ type Confirmation struct { // Typed, when set, must be typed to confirm, for irreversible actions such as deleting a repository. Typed string // NonTTY names the action in the error returned when no prompt is possible and --yes is - // missing: `delete label "bug" from alice/x`. + // missing: `delete label "bug" from alice/x`. Without it, a confirmation that defaults to no + // names the Prompt up to its question mark. NonTTY string } @@ -225,38 +196,25 @@ func ConfirmAction(ctx context.Context, c Confirmation, yes bool, streams Stream return nil } if !policy.CanPrompt(streams) { - action := "" - if c.NonTTY != "" { - action = " to " + c.NonTTY + action := c.NonTTY + if question, _, ok := strings.Cut(c.Prompt, "?"); action == "" && ok && !c.DefaultYes && question != "" { + action = strings.ToLower(question[:1]) + question[1:] + } + if action != "" { + action = " to " + action } return clierr.Wrap(clierr.Usage, fmt.Errorf("confirmation required%s; rerun with --yes", action)) } - asker, rich := prompt.(Asker) if c.Typed != "" { - title := c.Prompt + " Type " + c.Typed + " to confirm:" - matches := func(s string) bool { return strings.EqualFold(strings.TrimSpace(s), c.Typed) } - if rich { - _, err := asker.Ask(ctx, title, func(s string) error { - if !matches(s) { - return fmt.Errorf("type %s to confirm", c.Typed) - } - return nil - }) - return err - } - typed, err := prompt.Input(ctx, title) - if err == nil && !matches(typed) { - err = ErrCanceled - } + _, err := prompt.Ask(ctx, c.Prompt+" Type "+c.Typed+" to confirm:", func(s string) error { + if !strings.EqualFold(strings.TrimSpace(s), c.Typed) { + return fmt.Errorf("type %s to confirm", c.Typed) + } + return nil + }) return err } - var accepted bool - var err error - if rich { - accepted, err = asker.ConfirmDefault(ctx, c.Prompt, c.DefaultYes) - } else { - accepted, err = prompt.Confirm(ctx, c.Prompt) - } + accepted, err := prompt.ConfirmDefault(ctx, c.Prompt, c.DefaultYes) if err == nil && !accepted { err = ErrCanceled } diff --git a/internal/terminal/streams_test.go b/internal/terminal/streams_test.go index a80212b3..7e4fc8c7 100644 --- a/internal/terminal/streams_test.go +++ b/internal/terminal/streams_test.go @@ -31,15 +31,6 @@ func accessiblePrompter(t *testing.T, input string) (*FormPrompter, *bytes.Buffe func noEdit(context.Context, string) (Draft, error) { return Draft{}, nil } -type fakePrompt struct{ inputs int } - -func (f *fakePrompt) Input(context.Context, string) (string, error) { - f.inputs++ - return "provided", nil -} -func (f *fakePrompt) SecretInput(context.Context, string) (string, error) { return "", nil } -func (f *fakePrompt) Confirm(context.Context, string) (bool, error) { return false, nil } - func TestPromptPolicy(t *testing.T) { for _, tc := range []struct { policy Policy @@ -54,12 +45,12 @@ func TestPromptPolicy(t *testing.T) { } { require.Equal(t, tc.expected, tc.policy.CanPrompt(tc.streams)) } - prompt := &fakePrompt{} + prompt := &askingPrompt{} value := "supplied" asked, err := RequireInput(context.Background(), &value, "title", "title", nil, Streams{}, Policy{}, prompt) require.NoError(t, err) require.False(t, asked) - require.Zero(t, prompt.inputs) + require.Empty(t, prompt.titles) value = "" _, err = RequireInput(context.Background(), &value, "title", "title", nil, Streams{}, Policy{}, prompt) require.ErrorContains(t, err, "--title") @@ -68,7 +59,6 @@ func TestPromptPolicy(t *testing.T) { // askingPrompt answers Asker prompts from scripted answers. Ask skips answers that validate rejects, // as an inline re-prompt does, and cancels when none is left. type askingPrompt struct { - fakePrompt answers, rejected, titles []string accept bool defaults []bool @@ -87,6 +77,8 @@ func (p *askingPrompt) Ask(_ context.Context, title string, validate func(string return "", ErrCanceled } +func (p *askingPrompt) SecretInput(context.Context, string) (string, error) { return "", nil } + func (p *askingPrompt) AskText(_ context.Context, title, initial string) (string, error) { p.titles = append(p.titles, title) return initial + " edited", nil @@ -112,14 +104,13 @@ func TestConfirmAction(t *testing.T) { message string rejected []string }{ - {name: "accepted by flag", c: deleteLabel, yes: true, prompt: &fakePrompt{}}, - {name: "names the action without a terminal", c: deleteLabel, prompt: &fakePrompt{}, code: clierr.Usage, message: `confirmation required to delete label "bug" from alice/x; rerun with --yes`}, - {name: "without a terminal or action", c: Confirmation{Prompt: "Go?"}, prompt: &fakePrompt{}, code: clierr.Usage, message: "confirmation required; rerun with --yes"}, + {name: "accepted by flag", c: deleteLabel, yes: true, prompt: &askingPrompt{}}, + {name: "names the action without a terminal", c: deleteLabel, prompt: &askingPrompt{}, code: clierr.Usage, message: `confirmation required to delete label "bug" from alice/x; rerun with --yes`}, + {name: "names the prompted action without a terminal", c: Confirmation{Prompt: "Delete alice/x? It becomes read-only."}, prompt: &askingPrompt{}, code: clierr.Usage, message: "confirmation required to delete alice/x; rerun with --yes"}, + {name: "without a terminal or action", c: Confirmation{Prompt: "Go?", DefaultYes: true}, prompt: &askingPrompt{}, code: clierr.Usage, message: "confirmation required; rerun with --yes"}, {name: "accepted", c: deleteLabel, streams: terminal, prompt: &askingPrompt{accept: true}}, {name: "declined", c: deleteLabel, streams: terminal, prompt: &askingPrompt{}, code: clierr.Canceled, message: "canceled"}, - {name: "declined to a plain prompter", c: deleteLabel, streams: terminal, prompt: &fakePrompt{}, code: clierr.Canceled, message: "canceled"}, {name: "re-prompts until the name is typed", c: typed, streams: terminal, prompt: &askingPrompt{answers: []string{"alice/y", " Alice/X "}}, rejected: []string{"alice/y"}}, - {name: "typed wrong to a plain prompter", c: typed, streams: terminal, prompt: &fakePrompt{}, code: clierr.Canceled, message: "canceled"}, } { t.Run(tc.name, func(t *testing.T) { err := ConfirmAction(context.Background(), tc.c, tc.yes, tc.streams, Policy{Enabled: true}, tc.prompt) @@ -165,10 +156,6 @@ func TestRequireInput(t *testing.T) { require.Equal(t, []string{"Color (hex)"}, asker.titles) value = "" - _, err = RequireInput(context.Background(), &value, "color", "Color (hex)", hex, streams, policy, &fakePrompt{}) - require.Equal(t, clierr.Usage, clierr.ExitCode(err)) - require.EqualError(t, err, "--color: use six hex digits") - _, err = RequireInput(context.Background(), &value, "color", "Color (hex)", hex, Streams{}, policy, asker) require.EqualError(t, err, "missing required input: supply --color") } diff --git a/main_test.go b/main_test.go index 436712c8..845414a4 100644 --- a/main_test.go +++ b/main_test.go @@ -81,11 +81,21 @@ func TestSignalInterruptReturns130(t *testing.T) { type decliningPrompter struct{} -func (decliningPrompter) Input(context.Context, string) (string, error) { return "", nil } +func (decliningPrompter) Ask(context.Context, string, func(string) error) (string, error) { + return "", nil +} + +func (decliningPrompter) AskText(context.Context, string, string) (string, error) { + return "", nil +} + func (decliningPrompter) SecretInput(context.Context, string) (string, error) { return "", nil } -func (decliningPrompter) Confirm(context.Context, string) (bool, error) { return false, nil } + +func (decliningPrompter) ConfirmDefault(context.Context, string, bool) (bool, error) { + return false, nil +} func TestRunReportsDeclinedConfirmationsAndExplainedFailures(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {