From 1f4b5f868f4d58a8243e7f24382f23765804f817 Mon Sep 17 00:00:00 2001 From: bircni Date: Mon, 12 Oct 2026 10:47:54 +0200 Subject: [PATCH] refactor(prompts): fold Asker into Prompter and share delete confirmations FormPrompter is the only production prompter and implements the rich prompt API, so Ask, AskText and ConfirmDefault become the Prompter interface and the plain Input/Confirm fallbacks go away. Confirmations that default to no derive their non-terminal wording from the prompt, and single deletes share deleteConfirmed for the prompt, the request and the done line. Signed-off-by: bircni --- internal/app/action_values.go | 7 +- internal/app/actions.go | 13 +--- internal/app/actions_test.go | 3 - internal/app/admin_users.go | 2 +- internal/app/auth.go | 2 +- internal/app/branches.go | 4 +- internal/app/comments.go | 17 ++--- internal/app/editor.go | 4 +- internal/app/editor_test.go | 8 +-- internal/app/helpers_test.go | 33 ++++++++-- internal/app/issue_attachments.go | 9 +-- internal/app/label_options_test.go | 2 +- internal/app/labels.go | 10 +-- internal/app/metadata.go | 2 +- internal/app/prompts.go | 13 ++++ internal/app/prompts_test.go | 24 +------ internal/app/pull_merge_form_test.go | 4 +- internal/app/pull_review_form_test.go | 4 +- internal/app/pull_review_lifecycle.go | 11 +--- internal/app/pull_review_test.go | 2 +- internal/app/releases.go | 2 +- internal/app/repo_mutations.go | 2 +- internal/app/ssh_keys.go | 9 +-- internal/app/target.go | 2 +- internal/app/teams.go | 14 ++-- internal/app/time.go | 20 ++---- internal/app/webhooks.go | 13 +--- internal/app/wiki.go | 11 +--- internal/terminal/streams.go | 94 ++++++++------------------- internal/terminal/streams_test.go | 29 +++------ main_test.go | 14 +++- 31 files changed, 131 insertions(+), 253 deletions(-) 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) {