From ae0ea27bb0379ddea01161ecb3666b458b80051b Mon Sep 17 00:00:00 2001 From: bircni Date: Mon, 12 Oct 2026 00:19:35 +0200 Subject: [PATCH] fix: reconcile the issues stream and remove the legacy output shims Use one canPrompt and pullNumberArguments for the comment commands, set display preferences instead of the retired environment variables in tests, and delete writeList, writeItem, batch, table, writeRecord(s), the legacy column map, confirm, require, validateMilestoneFlags and listWindow now that every command uses their replacements. Signed-off-by: bircni --- internal/app/app.go | 18 ---- internal/app/comments.go | 5 +- internal/app/comments_test.go | 4 +- internal/app/discussion_search_test.go | 3 +- internal/app/helpers_test.go | 9 ++ internal/app/human_output.go | 110 --------------------- internal/app/issue_attachments_test.go | 3 +- internal/app/issue_edit.go | 5 +- internal/app/issues.go | 22 +---- internal/app/labels.go | 5 +- internal/app/metadata.go | 1 - internal/app/milestone_create_form_test.go | 3 +- internal/services/helpers.go | 7 -- 13 files changed, 23 insertions(+), 172 deletions(-) diff --git a/internal/app/app.go b/internal/app/app.go index bc115157..eaec9e16 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -407,24 +407,6 @@ func acceptsArgs(args []string) bool { return err == nil && cmd.ParseFlags(rest) == nil && cmd.ValidateArgs(cmd.Flags().Args()) == nil } -// confirm asks for confirmation under the prompt policy unless --yes was supplied. -func (a *application) confirm(cmd *cobra.Command, description string) error { - s, err := a.deps.Settings.Load() - if err != nil { - return err - } - return terminal.Confirm(cmd.Context(), description, a.options.yes, a.deps.Streams, a.policy(s), a.deps.Prompter) -} - -// require prompts for a missing required value under the prompt policy. -func (a *application) require(cmd *cobra.Command, value *string, flag string) (bool, error) { - s, err := a.deps.Settings.Load() - if err != nil { - return false, err - } - return terminal.Require(cmd.Context(), value, flag, a.deps.Streams, a.policy(s), a.deps.Prompter) -} - func (a *application) policy(snapshot settings.Snapshot) terminal.Policy { return terminal.Policy{Enabled: snapshot.Preferences.Prompt == "enabled", NoInput: a.options.noInput, Machine: a.options.json != "", Interactive: a.options.interactive} } diff --git a/internal/app/comments.go b/internal/app/comments.go index 6df0786d..32e97a07 100644 --- a/internal/app/comments.go +++ b/internal/app/comments.go @@ -4,7 +4,6 @@ package app import ( - "cmp" "errors" "fmt" "strconv" @@ -113,8 +112,8 @@ func (a *application) readComment(cmd *cobra.Command, f commentFlags, initial st // promptComment asks for a comment in the configured editor, or in a multi-line field without one. func (a *application) promptComment(cmd *cobra.Command, initial string) (string, terminal.Draft, error) { - if prompt, err := a.canPrompt(); err != nil || !prompt { - return "", terminal.Draft{}, cmp.Or(err, clierr.Wrap(clierr.Usage, errors.New("missing required input: supply --body, --body-file or --editor"))) + if !a.canPrompt() { + return "", terminal.Draft{}, clierr.Wrap(clierr.Usage, errors.New("missing required input: supply --body, --body-file or --editor")) } if preferences, err := a.preferences(); err == nil && preferences.Editor != "" { draft, err := a.editDraft(cmd, initial) diff --git a/internal/app/comments_test.go b/internal/app/comments_test.go index 947b030b..03190aa5 100644 --- a/internal/app/comments_test.go +++ b/internal/app/comments_test.go @@ -33,6 +33,7 @@ func TestCommentConfirmationNamesTheTarget(t *testing.T) { defer server.Close() deps, out, diagnostics := serverDeps(t, server.URL) deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1"}) + plainOutput(t, deps) prompt := &scriptedPrompter{accept: true, input: "Prompted"} deps.Prompter = prompt deps.Streams.InTTY, deps.Streams.ErrTTY, deps.Streams.OutTTY = true, true, true @@ -160,7 +161,8 @@ func TestCommentGroupCommentsLikeGh(t *testing.T) { "id:\t80\nauthor:\tbob\ncreated_at:\t2026-10-11T19:00:00Z\nurl:\t\n--\nSecond\nline\n--\n"+ "id:\t81\nauthor:\tbob\ncreated_at:\t2026-10-11T19:10:00Z\nurl:\t\n--\nThird\n--\n", out.String()) deps.Streams.OutTTY = true - deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1", "TEA_MARKDOWN": "raw"}) + deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1"}) + plainOutput(t, deps) require.NoError(t, run("issue", "comment", "list", "1")) require.Equal(t, "#79 alice • about 2 hours ago\n First **one**\n\n"+ "#80 bob • about 1 hour ago (edited)\n Second\n line\n\n"+ diff --git a/internal/app/discussion_search_test.go b/internal/app/discussion_search_test.go index ff7ad3eb..ae206be9 100644 --- a/internal/app/discussion_search_test.go +++ b/internal/app/discussion_search_test.go @@ -82,7 +82,8 @@ func TestSearchTables(t *testing.T) { defer server.Close() deps, out, diagnostics := serverDeps(t, server.URL) deps.Now = func() time.Time { return time.Date(2026, 10, 11, 20, 0, 0, 0, time.UTC) } - deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1", "TEA_HYPERLINKS": "never"}) + deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1"}) + plainOutput(t, deps) deps.Streams.OutTTY = true require.NoError(t, Execute(context.Background(), deps, []string{"search", "prs", "login", "-s", "all", "-L", "2", "--author", "@me"})) host := strings.TrimPrefix(server.URL, "http://") diff --git a/internal/app/helpers_test.go b/internal/app/helpers_test.go index 4b104e65..3e3ab191 100644 --- a/internal/app/helpers_test.go +++ b/internal/app/helpers_test.go @@ -308,3 +308,12 @@ func rawMarkdown(t *testing.T, deps Dependencies) { return nil })) } + +// plainOutput keeps terminal output free of rendered Markdown and hyperlinks so it compares as text. +func plainOutput(t *testing.T, deps Dependencies) { + t.Helper() + require.NoError(t, deps.Settings.Update(settings.TargetPreferences, func(s *settings.Snapshot) error { + s.Preferences.Markdown, s.Preferences.Hyperlinks = "raw", "never" + return nil + })) +} diff --git a/internal/app/human_output.go b/internal/app/human_output.go index 4b79b4e0..608aa9d6 100644 --- a/internal/app/human_output.go +++ b/internal/app/human_output.go @@ -6,13 +6,10 @@ package app import ( "errors" "fmt" - "slices" - "strings" "gitea.dev/tea/internal/services" "gitea.dev/tea/internal/settings" "gitea.dev/tea/internal/terminal" - "github.com/mattn/go-runewidth" "github.com/spf13/cobra" ) @@ -209,19 +206,6 @@ func writeBatchJSON[T any](a *application, cmd *cobra.Command, selections int, r return nil } -// batch applies run to each selection like batchDone, for commands that predate it: its human -// output is one table of the results. -func batch[K, T any](a *application, cmd *cobra.Command, keys []K, label func(K) string, run func(K) (T, error), row func(T) []terminal.Cell) error { - results, failures := applyBatch(cmd, keys, label, run, func(T) {}) - var err error - if a.options.json == "" { - err = a.writeHuman(cmd, legacyTable(a, cmd, results, row)) - } else { - err = writeBatchJSON(a, cmd, len(keys), results) - } - return errors.Join(append(failures, err)...) -} - // batchDone applies a mutation to each selection like batch, reporting each success with a done // line from report, such as "Closed issue alice/x#5 (Fix login)", and the results as JSON. func batchDone[K, T any](a *application, cmd *cobra.Command, keys []K, label func(K) string, run func(K) (T, error), report func(T) string) error { @@ -233,103 +217,9 @@ func batchDone[K, T any](a *application, cmd *cobra.Command, keys []K, label fun return errors.Join(append(failures, err)...) } -// writeList writes items as a JSON array, or as one table through the pager. List commands move -// to writeTable. -func writeList[T any](a *application, cmd *cobra.Command, items []T, row func(T) []terminal.Cell) error { - if a.options.json != "" { - return a.writeJSON(cmd, items) - } - return a.writeHuman(cmd, legacyTable(a, cmd, items, row)) -} - -// writeItem writes one item as a JSON object, or as its list row. Views move to writeView, and -// mutations to done and printURL. -func writeItem[T any](a *application, cmd *cobra.Command, item T, row func(T) []terminal.Cell) error { - if a.options.json != "" { - return a.writeJSON(cmd, item) - } - return a.writeHuman(cmd, legacyTable(a, cmd, []T{item}, row)) -} - -// legacyColumns gives lists that still render through writeList a header row and a flexible -// column, keyed by command path without the root. Their owners replace an entry with a listSpec. -var legacyColumns = map[string][]terminal.Column{} - -// numberedColumns head the rows of issueRow and pullRow. -var numberedColumns = []terminal.Column{{Header: "ID", Prefix: "#"}, {Header: "Title", Flex: true}, {Header: "State"}, {Header: "Author"}} - -// legacyTable renders rows of commands that predate listSpec: tab-separated when piped, and on a -// TTY fit to the terminal width by truncating their widest column, unless legacyColumns names -// their columns. -func legacyTable[T any](a *application, cmd *cobra.Command, items []T, row func(T) []terminal.Cell) string { - rows := make([][]terminal.Cell, len(items)) - for i, item := range items { - rows[i] = row(item) - } - columns, ok := legacyColumns[strings.TrimPrefix(cmd.CommandPath(), cmd.Root().Name()+" ")] - if !ok { - columns = widestFlex(rows) - } - return terminal.Table(columns, rows, a.display()) -} - -// widestFlex makes the column with the widest cell flexible. -func widestFlex(rows [][]terminal.Cell) []terminal.Column { - var widths []int - for _, row := range rows { - for i, cell := range row { - if i == len(widths) { - widths = append(widths, 0) - } - widths[i] = max(widths[i], runewidth.StringWidth(terminal.SafeLine(cell.Text))) - } - } - columns := make([]terminal.Column, len(widths)) - if len(widths) > 0 { - columns[slices.Index(widths, slices.Max(widths))].Flex = true - } - return columns -} - -// writeRecord writes one item as a JSON object, or as a human-readable record. -func writeRecord[T any](a *application, cmd *cobra.Command, item T, record func(T) terminal.Record) error { - if a.options.json != "" { - return a.writeJSON(cmd, item) - } - return a.writeHuman(cmd, record(item).Render(a.colorEnabled())) -} - -// writeRecords writes items as a JSON array, or as consecutive human-readable records. -func writeRecords[T any](a *application, cmd *cobra.Command, items []T, record func(T) terminal.Record) error { - if a.options.json != "" { - return a.writeJSON(cmd, items) - } - color := a.colorEnabled() - var text strings.Builder - for _, item := range items { - text.WriteString(record(item).Render(color)) - } - return a.writeHuman(cmd, text.String()) -} - -// numberedRecord heads an issue or pull request with its emphasized number and title. -func numberedRecord(number int64, title, url string) terminal.Record { - return terminal.Record{Heading: []terminal.Cell{{Text: fmt.Sprintf("#%d", number), Bold: true}, {Text: title}}, Lines: []string{url}} -} - // stateColors colors issue, pull request and milestone states with the terminal's own palette. var stateColors = map[string]string{"open": "green", "closed": "red", "merged": "magenta", "draft": "muted"} -// table renders rows aligned like a TTY table without header or truncation, for commands that -// predate listSpec and compose their own output. -func table[T any](items []T, row func(T) []terminal.Cell, color bool) string { - rows := make([][]terminal.Cell, len(items)) - for i, item := range items { - rows[i] = row(item) - } - return terminal.Table(nil, rows, terminal.Display{TTY: true, Color: color, TrueColor: true, Links: color}) -} - // morePagesHint tells human readers on stderr that the listing continues past this page. func (a *application) morePagesHint(cmd *cobra.Command, more bool, page int) error { if !more || a.options.json != "" { diff --git a/internal/app/issue_attachments_test.go b/internal/app/issue_attachments_test.go index 9bf111d9..f1590387 100644 --- a/internal/app/issue_attachments_test.go +++ b/internal/app/issue_attachments_test.go @@ -106,7 +106,8 @@ func TestAttachmentTablesDownloadsAndDoneLines(t *testing.T) { defer server.Close() deps, out, diagnostics := serverDeps(t, server.URL) deps.Now = func() time.Time { return time.Date(2026, 10, 11, 20, 0, 0, 0, time.UTC) } - deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1", "TEA_HYPERLINKS": "never"}) + deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1"}) + plainOutput(t, deps) run := func(tty bool, args ...string) error { out.Reset() diagnostics.Reset() diff --git a/internal/app/issue_edit.go b/internal/app/issue_edit.go index 5b11b19e..8cddb57e 100644 --- a/internal/app/issue_edit.go +++ b/internal/app/issue_edit.go @@ -32,10 +32,7 @@ $ tea issue edit 12 --body-file notes.md`, Annotations: map[string]string{"tea.a if err := validateBodySources(cmd); err != nil { return err } - prompt, err := a.canPrompt() - if err != nil { - return err - } + prompt := a.canPrompt() if !localFlagsChanged(cmd) { if !prompt || a.deps.IssueEditForm == nil { return errNothingToEdit diff --git a/internal/app/issues.go b/internal/app/issues.go index 9ff10b56..7f6722bf 100644 --- a/internal/app/issues.go +++ b/internal/app/issues.go @@ -244,10 +244,7 @@ $ tea issue create --title "Idea" --web`, Args: argsExact(0), RunE: func(cmd *co return err } } - prompt, err := a.canPrompt() - if err != nil { - return err - } + prompt := a.canPrompt() if !localFlagsChanged(cmd) && prompt && a.deps.IssueForm != nil { return a.createIssueInteractively(cmd) } @@ -491,23 +488,6 @@ func milestoneID(cmd *cobra.Command, t target, raw string) (int64, error) { return milestone.ID, err } -// validateMilestoneFlags checks the --milestone ID and --milestone-name flags of pr create and pr -// edit before any request. -func validateMilestoneFlags(cmd *cobra.Command, name string, id int64) error { - if cmd.Flags().Changed("milestone-name") { - if cmd.Flags().Changed("milestone") { - return clierr.Wrap(clierr.Usage, errors.New("--milestone and --milestone-name are mutually exclusive")) - } - if strings.TrimSpace(name) == "" { - return clierr.Wrap(clierr.Usage, errors.New("--milestone-name cannot be empty; use --milestone=0 for none")) - } - } - if id < 0 { - return clierr.Wrap(clierr.Usage, errors.New("--milestone must be a positive ID, or 0 for none")) - } - return nil -} - func milestoneChoices(items []services.Milestone) []terminal.IssueMilestoneChoice { choices := make([]terminal.IssueMilestoneChoice, len(items)) for i, item := range items { diff --git a/internal/app/labels.go b/internal/app/labels.go index ebf3f4f0..e9e10aa8 100644 --- a/internal/app/labels.go +++ b/internal/app/labels.go @@ -247,10 +247,7 @@ $ tea label edit old-stuff --archived`, Annotations: map[string]string{"tea.argu if err != nil { return err } - prompt, err := a.canPrompt() - if err != nil { - return err - } + prompt := a.canPrompt() form, hasForm := a.deps.Prompter.(terminal.LabelForm) if opts == (services.LabelOptions{}) && (!prompt || !hasForm) { return errNothingToEdit diff --git a/internal/app/metadata.go b/internal/app/metadata.go index df26599f..e2a3f28b 100644 --- a/internal/app/metadata.go +++ b/internal/app/metadata.go @@ -222,7 +222,6 @@ func formOptions(values terminal.MilestoneFormValues) services.MilestoneOptions return services.MilestoneOptions{Title: &values.Title, Description: &values.Description, Deadline: &normalized} } - func (a *application) milestoneCreateCommand(service milestoneService) *cobra.Command { var fields milestoneFields var state string diff --git a/internal/app/milestone_create_form_test.go b/internal/app/milestone_create_form_test.go index 4fcbd6c0..23b76f38 100644 --- a/internal/app/milestone_create_form_test.go +++ b/internal/app/milestone_create_form_test.go @@ -89,7 +89,8 @@ func TestMilestoneTablesViewsAndDoneLines(t *testing.T) { defer server.Close() deps, out, diagnostics := serverDeps(t, server.URL) deps.Now = func() time.Time { return time.Date(2026, 10, 11, 20, 0, 0, 0, time.UTC) } - deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1", "TEA_MARKDOWN": "raw", "TEA_HYPERLINKS": "never"}) + deps.Getenv = serverEnv(server.URL, map[string]string{"NO_COLOR": "1"}) + plainOutput(t, deps) run := func(tty bool, args ...string) { t.Helper() out.Reset() diff --git a/internal/services/helpers.go b/internal/services/helpers.go index e1fcba83..8f99edf1 100644 --- a/internal/services/helpers.go +++ b/internal/services/helpers.go @@ -64,13 +64,6 @@ type Page[T any] struct { Counted bool } -// listWindow returns items [(page-1)*limit, page*limit) even when the server caps its page size -// below limit (MAX_RESPONSE_ITEMS), and reports whether the collection continues past them. -func listWindow[T any](limit, page int, fetch func(limit, page int) ([]T, http.Header, error)) ([]T, bool, error) { - window, err := listPage(limit, page, fetch) - return window.Items, window.More, err -} - // listPage is listWindow that also reports the collection size the server counted. func listPage[T any](limit, page int, fetch func(limit, page int) ([]T, http.Header, error)) (Page[T], error) { items, headers, err := fetch(limit, page)