From 83924892ffba884a8c7fc77895e2521f6aa14769 Mon Sep 17 00:00:00 2001 From: bircni Date: Mon, 12 Oct 2026 11:22:52 +0200 Subject: [PATCH] refactor: reuse the existing service and rendering helpers services.IsNotFound replaces the hand-rolled 404 checks, convertPage the copied page conversions and lastPathNumber the comment's own number parser. The List wrappers around ListPage of releases, branches, wiki pages, secrets and variables go, pull request filters resolve milestones through Milestones.FindOne like issues, and localFlagsChanged, boolOrder, Cell.Render, stateBadge and dueText replace their copies. Signed-off-by: bircni --- internal/app/action_runs.go | 11 +---- internal/app/actions.go | 12 +++--- internal/app/args.go | 8 ++-- internal/app/auth.go | 12 +----- internal/app/completion.go | 16 ++++---- internal/app/issues.go | 4 +- internal/app/metadata.go | 7 ++-- internal/app/pull_create.go | 2 +- internal/app/pull_edit.go | 2 +- internal/app/pulls.go | 40 +------------------ internal/app/wiki.go | 4 +- internal/services/action_logs.go | 2 +- internal/services/action_values.go | 20 ++-------- internal/services/action_values_test.go | 8 ++-- internal/services/branches.go | 12 ++---- internal/services/branches_test.go | 4 +- internal/services/comments.go | 12 +----- internal/services/convert.go | 5 +++ internal/services/errors.go | 6 +++ internal/services/issue_dependencies.go | 5 +-- internal/services/issues.go | 2 +- internal/services/metadata.go | 4 +- internal/services/milestone_selection.go | 27 ------------- internal/services/milestone_selection_test.go | 10 ++--- internal/services/organizations.go | 15 +------ internal/services/pulls.go | 4 +- internal/services/releases.go | 10 +---- internal/services/releases_test.go | 4 +- internal/services/repositories.go | 2 +- internal/services/repository_search.go | 2 +- internal/services/wiki.go | 12 +----- internal/services/wiki_test.go | 10 ++--- internal/terminal/table.go | 3 ++ 33 files changed, 85 insertions(+), 212 deletions(-) diff --git a/internal/app/action_runs.go b/internal/app/action_runs.go index 3542fa8d..1c358d99 100644 --- a/internal/app/action_runs.go +++ b/internal/app/action_runs.go @@ -262,7 +262,7 @@ func renderRun(run services.Run, d terminal.Display, hints []string) string { if !d.TTY { return runRecord(run, d, now) } - style := func(cell terminal.Cell) string { return paintActionCell(cell, d.Color) } + style := func(cell terminal.Cell) string { return cell.Render(d) } muted := func(text string) string { return style(terminal.Cell{Text: text, Muted: true}) } var b strings.Builder heading := run.Workflow @@ -321,15 +321,6 @@ func renderRun(run services.Run, d terminal.Display, hints []string) string { return b.String() } -// paintActionCell styles a cell's safe text outside a table: its color, muted or bold. -func paintActionCell(cell terminal.Cell, color bool) string { - name := cell.Color - if cell.Muted { - name = "muted" - } - return terminal.Emphasis(terminal.Colorize(name, terminal.SafeLine(cell.Text), color), cell.Bold && color) -} - // jobLines lists jobs, each followed by its steps, for the run view. func jobLines(jobs []services.Job, now time.Time) [][]terminal.Cell { var rows [][]terminal.Cell diff --git a/internal/app/actions.go b/internal/app/actions.go index 30da3526..5ab6262f 100644 --- a/internal/app/actions.go +++ b/internal/app/actions.go @@ -8,7 +8,6 @@ import ( "context" "errors" "fmt" - "net/http" "slices" "strconv" "strings" @@ -17,7 +16,6 @@ import ( "gitea.dev/tea/internal/clierr" "gitea.dev/tea/internal/services" "gitea.dev/tea/internal/terminal" - "gitea.dev/tea/internal/transport" "github.com/spf13/cobra" ) @@ -144,11 +142,11 @@ func (a *application) renderWorkflow(t target, workflow services.Workflow, runs fields := []terminal.Field{{Label: "File", Value: workflow.ID}, {Label: "Path", Value: workflow.Path}, {Label: "State", Value: workflow.State}} return terminal.View{Title: workflow.Name, Fields: fields, URL: workflow.URL, Body: terminal.Table(runColumns, rows, d)}.Render(d) } - muted := func(text string) string { return paintActionCell(terminal.Cell{Text: text, Muted: true}, d.Color) } + muted := func(text string) string { return terminal.Cell{Text: text, Muted: true}.Render(d) } var b strings.Builder - b.WriteString(paintActionCell(terminal.Cell{Text: workflow.Name, Bold: true}, d.Color) + " " + muted(workflow.ID) + "\n") - b.WriteString(paintActionCell(workflowState(workflow), d.Color) + " • " + muted(workflow.Path) + "\n\n") - b.WriteString(paintActionCell(terminal.Cell{Text: "RECENT RUNS", Bold: true}, d.Color) + "\n") + b.WriteString(terminal.Cell{Text: workflow.Name, Bold: true}.Render(d) + " " + muted(workflow.ID) + "\n") + b.WriteString(workflowState(workflow).Render(d) + " • " + muted(workflow.Path) + "\n\n") + b.WriteString(terminal.Cell{Text: "RECENT RUNS", Bold: true}.Render(d) + "\n") if len(runs) == 0 { b.WriteString(muted("No runs yet") + "\n") } else { @@ -315,7 +313,7 @@ func (a *application) dispatchRef(ctx context.Context, t target, ref string) (st return defaultBranch() } _, err = services.Branches{Client: t.client}.View(ctx, t.Repo, branch) - if api, ok := errors.AsType[*transport.APIError](err); ok && api.Status == http.StatusNotFound { + if services.IsNotFound(err) { fallback, err := defaultBranch() if err != nil { return "", err diff --git a/internal/app/args.go b/internal/app/args.go index 2d10f080..68f87259 100644 --- a/internal/app/args.go +++ b/internal/app/args.go @@ -307,9 +307,11 @@ func argumentName(cmd *cobra.Command, argument string) string { return noun + " " + text } -// localFlagsChanged reports whether any command-specific flag was supplied. -func localFlagsChanged(cmd *cobra.Command) bool { +// localFlagsChanged reports whether a command-specific flag other than those in except was supplied. +func localFlagsChanged(cmd *cobra.Command, except ...string) bool { changed := false - cmd.LocalNonPersistentFlags().VisitAll(func(flag *pflag.Flag) { changed = changed || flag.Changed }) + cmd.LocalNonPersistentFlags().VisitAll(func(flag *pflag.Flag) { + changed = changed || flag.Changed && !slices.Contains(except, flag.Name) + }) return changed } diff --git a/internal/app/auth.go b/internal/app/auth.go index 415103de..6312bec9 100644 --- a/internal/app/auth.go +++ b/internal/app/auth.go @@ -463,7 +463,7 @@ $ tea auth status --host gitea.com --json`, rows = append(rows, row) } slices.SortFunc(rows, func(x, y authStatusRow) int { - return cmp.Or(strings.Compare(x.Host, y.Host), -compareBool(x.environment(), y.environment()), strings.Compare(x.User, y.User)) + return cmp.Or(strings.Compare(x.Host, y.Host), cmp.Compare(boolOrder(y.environment()), boolOrder(x.environment())), strings.Compare(x.User, y.User)) }) if a.options.json != "" { return a.writeJSON(cmd, rows) @@ -487,16 +487,6 @@ $ tea auth status --host gitea.com --json`, return status } -func compareBool(x, y bool) int { - switch { - case x == y: - return 0 - case x: - return 1 - } - return -1 -} - // accountStatus reports a stored account without revealing its credential. func (a *application) accountStatus(ctx context.Context, s settings.Snapshot, instance, user string, verify bool) authStatusRow { host := s.Hosts.Hosts[instance] diff --git a/internal/app/completion.go b/internal/app/completion.go index ecf53f2b..873c2c5f 100644 --- a/internal/app/completion.go +++ b/internal/app/completion.go @@ -385,8 +385,8 @@ func (a *application) completeUsers() cobra.CompletionFunc { // completeBranches completes the local branches, then the repository's branches. func (a *application) completeBranches() cobra.CompletionFunc { return mergeCompletions(a.completeLocalBranches, a.apiCompletion(true, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { - items, err := services.Branches{Client: t.client}.List(ctx, t.Repo, completionLimit, 1) - return describe(items, func(item services.Branch) (string, string) { + items, err := services.Branches{Client: t.client}.ListPage(ctx, t.Repo, completionLimit, 1) + return describe(items.Items, func(item services.Branch) (string, string) { if item.Protected { return item.Name, "protected" } @@ -406,8 +406,8 @@ func (a *application) completeTags() cobra.CompletionFunc { // completeReleases completes release tags with their titles. func (a *application) completeReleases() cobra.CompletionFunc { return a.apiCompletion(true, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { - items, err := services.Releases{Client: t.client, Repository: t.Repo}.List(ctx, completionLimit, 1) - return describe(items, func(item services.Release) (string, string) { return item.Tag, item.Title }), err + items, err := services.Releases{Client: t.client, Repository: t.Repo}.ListPage(ctx, completionLimit, 1) + return describe(items.Items, func(item services.Release) (string, string) { return item.Tag, item.Title }), err }) } @@ -493,8 +493,8 @@ func (a *application) completeRuns() cobra.CompletionFunc { // organization or account selects the repository. func (a *application) completeSecrets(scope func() services.ActionScope) cobra.CompletionFunc { return a.apiCompletion(true, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { - items, err := services.ActionSecrets{Client: t.client, Scope: actionScope(scope, t)}.List(ctx, completionLimit, 1) - return describe(items, func(item services.ActionSecret) (string, string) { return item.Name, item.Description }), err + items, err := services.ActionSecrets{Client: t.client, Scope: actionScope(scope, t)}.ListPage(ctx, completionLimit, 1) + return describe(items.Items, func(item services.ActionSecret) (string, string) { return item.Name, item.Description }), err }) } @@ -502,8 +502,8 @@ func (a *application) completeSecrets(scope func() services.ActionScope) cobra.C // an organization or account selects the repository. func (a *application) completeVariables(scope func() services.ActionScope) cobra.CompletionFunc { return a.apiCompletion(true, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { - items, err := services.ActionVariables{Client: t.client, Scope: actionScope(scope, t)}.List(ctx, completionLimit, 1) - return describe(items, func(item services.ActionVariable) (string, string) { return item.Name, item.Description }), err + items, err := services.ActionVariables{Client: t.client, Scope: actionScope(scope, t)}.ListPage(ctx, completionLimit, 1) + return describe(items.Items, func(item services.ActionVariable) (string, string) { return item.Name, item.Description }), err }) } diff --git a/internal/app/issues.go b/internal/app/issues.go index 6a9544f2..dee41f24 100644 --- a/internal/app/issues.go +++ b/internal/app/issues.go @@ -8,7 +8,6 @@ import ( "context" "errors" "fmt" - "net/http" "net/url" "path/filepath" "strconv" @@ -18,7 +17,6 @@ import ( "gitea.dev/tea/internal/clierr" "gitea.dev/tea/internal/services" "gitea.dev/tea/internal/terminal" - "gitea.dev/tea/internal/transport" "github.com/spf13/cobra" ) @@ -490,7 +488,7 @@ func issueLabel(n int64) string { return fmt.Sprintf("issue %d", n) } // itemError words the failure of one item of a batch, which the batch prefixes with the item, such // as "issue 9: not found in alice/x". func itemError(err error, scope string) error { - if apiErr, ok := errors.AsType[*transport.APIError](err); ok && apiErr.Status == http.StatusNotFound { + if services.IsNotFound(err) { return explain(err, "not found in %s", scope) } return err diff --git a/internal/app/metadata.go b/internal/app/metadata.go index a98605af..0e3dcc30 100644 --- a/internal/app/metadata.go +++ b/internal/app/metadata.go @@ -125,13 +125,12 @@ func milestoneView(t target, now time.Time) func(services.Milestone) terminal.Vi } due := "" if item.Deadline != nil { - deadline := terminal.ParseTime(*item.Deadline) - due = terminal.Timestamp(deadline) - meta = append([]string{"due " + deadline.Format("Jan 2, 2006") + " (" + terminal.RelativeTime(now, deadline) + ")"}, meta...) + due = terminal.Timestamp(terminal.ParseTime(*item.Deadline)) + meta = append([]string{"due " + dueText(now, *item.Deadline)}, meta...) } return terminal.View{ Title: item.Title, ID: "#" + strconv.FormatInt(item.ID, 10), - Badges: []terminal.Cell{{Text: titleCase(item.State), Color: stateColors[item.State]}}, Meta: strings.Join(meta, " • "), + Badges: []terminal.Cell{stateBadge(item.State)}, Meta: strings.Join(meta, " • "), Fields: []terminal.Field{ {Label: "State", Value: item.State, PipedOnly: true}, {Label: "Due", Value: due, PipedOnly: true}, diff --git a/internal/app/pull_create.go b/internal/app/pull_create.go index 40008ba8..37861f0d 100644 --- a/internal/app/pull_create.go +++ b/internal/app/pull_create.go @@ -90,7 +90,7 @@ is not on the remote yet is pushed first, with its upstream set.`, Args: argsExa return err } if milestone != "" && milestone != "0" { - if options.Milestone, err = milestoneFilter(cmd.Context(), t, milestone); err != nil { + if options.Milestone, err = milestoneID(cmd, t, milestone); err != nil { return err } } diff --git a/internal/app/pull_edit.go b/internal/app/pull_edit.go index d0ec3ec7..10ae2fd9 100644 --- a/internal/app/pull_edit.go +++ b/internal/app/pull_edit.go @@ -85,7 +85,7 @@ pull request, or mark it as a draft or ready for review.`, Annotations: pullArgu var none int64 opts.Milestone = &none case milestone != "": - id, err := milestoneFilter(cmd.Context(), t, milestone) + id, err := milestoneID(cmd, t, milestone) if err != nil { return err } diff --git a/internal/app/pulls.go b/internal/app/pulls.go index 8ae2f527..115c047e 100644 --- a/internal/app/pulls.go +++ b/internal/app/pulls.go @@ -4,9 +4,7 @@ package app import ( - "context" "errors" - "net/http" "slices" "strconv" "strings" @@ -14,9 +12,7 @@ import ( "gitea.dev/tea/internal/clierr" "gitea.dev/tea/internal/services" "gitea.dev/tea/internal/terminal" - "gitea.dev/tea/internal/transport" "github.com/spf13/cobra" - "github.com/spf13/pflag" ) func (a *application) prCommand() *cobra.Command { @@ -71,7 +67,7 @@ each pull request, which tea only makes when --json names them.`, Args: argsExac return err } if milestone != "" { - if options.Milestone, err = milestoneFilter(cmd.Context(), t, milestone); err != nil { + if options.Milestone, err = milestoneID(cmd, t, milestone); err != nil { return err } } @@ -91,7 +87,7 @@ each pull request, which tea only makes when --json names them.`, Args: argsExac noun := map[string]string{"open": "open pull requests", "closed": "closed pull requests", "merged": "merged pull requests", "all": "pull requests"}[state] what := noun + " in " + t.Repo empty := "no " + what - if filtered(cmd, "state", "limit", "page", "sort") { + if localFlagsChanged(cmd, "state", "limit", "page", "sort") { empty = "no " + noun + " match your filters in " + t.Repo } columns := []terminal.Column{{Header: "ID", Prefix: "#"}, {Header: "Title", Flex: true}, {Header: "Branch", Flex: true, Min: 12}, {Header: "State", PipedOnly: state == "open"}, {Header: "Author"}, {Header: "Created"}} @@ -137,22 +133,6 @@ $ tea pr list --json number,title,head --jq '.[] | select(.head == "feature/logi return cmd } -// changedFlags lists the names of the command's own flags that were given. -func changedFlags(cmd *cobra.Command) []string { - var names []string - cmd.LocalNonPersistentFlags().VisitAll(func(flag *pflag.Flag) { - if flag.Changed { - names = append(names, flag.Name) - } - }) - return names -} - -// filtered reports whether a flag other than those named, which do not filter, was given. -func filtered(cmd *cobra.Command, unfiltered ...string) bool { - return slices.ContainsFunc(changedFlags(cmd), func(name string) bool { return !slices.Contains(unfiltered, name) }) -} - // jsonSelects reports whether --json names one of fields, rather than selecting every field. func (a *application) jsonSelects(fields ...string) bool { for field := range strings.SplitSeq(a.options.json, ",") { @@ -163,22 +143,6 @@ func (a *application) jsonSelects(fields ...string) bool { return false } -// milestoneFilter resolves a milestone given as an ID or a title (D10): a number is an ID first, -// then a title. -func milestoneFilter(ctx context.Context, t target, raw string) (int64, error) { - service := services.Milestones{Client: t.client, Repository: t.Repo} - if id, err := strconv.ParseInt(raw, 10, 64); err == nil && id > 0 { - _, err := service.View(ctx, id) - if err == nil { - return id, nil - } - if apiErr, ok := errors.AsType[*transport.APIError](err); !ok || apiErr.Status != http.StatusNotFound { - return 0, err - } - } - return service.ResolveName(ctx, raw) -} - func (a *application) pullViewCommand() *cobra.Command { var web, comments bool cmd := &cobra.Command{Use: "view [ | | ]", Short: "View a pull request", Annotations: pullArgumentsAnnotation(nil), Args: usageArgs(cobra.MaximumNArgs(1)), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/internal/app/wiki.go b/internal/app/wiki.go index 9d9a41c6..135464c1 100644 --- a/internal/app/wiki.go +++ b/internal/app/wiki.go @@ -29,8 +29,8 @@ func (a *application) wikiCommand() *cobra.Command { // completeWikiPages completes the titles of the repository's wiki pages. func (a *application) completeWikiPages() cobra.CompletionFunc { return a.apiCompletion(true, func(ctx context.Context, t target, _ string) ([]cobra.Completion, error) { - pages, err := services.Wiki{Client: t.client, Repository: t.Repo}.List(ctx, completionLimit, 1) - return describe(pages, func(page services.WikiPage) (string, string) { return page.Title, "" }), err + pages, err := services.Wiki{Client: t.client, Repository: t.Repo}.ListPage(ctx, completionLimit, 1) + return describe(pages.Items, func(page services.WikiPage) (string, string) { return page.Title, "" }), err }) } diff --git a/internal/services/action_logs.go b/internal/services/action_logs.go index cab31796..25e265b3 100644 --- a/internal/services/action_logs.go +++ b/internal/services/action_logs.go @@ -82,7 +82,7 @@ func (s Actions) JobsPage(ctx context.Context, run int64, limit, page int) (Page if err != nil { return Page[Job]{}, err } - return Page[Job]{Items: convertAll(window.Items, jobFrom), More: window.More, Total: window.Total, Counted: window.Counted}, nil + return convertPage(window, jobFrom), nil } // WaitForFirstJob polls a freshly dispatched run until the server has created its first job. diff --git a/internal/services/action_values.go b/internal/services/action_values.go index 3d81888c..a0dece53 100644 --- a/internal/services/action_values.go +++ b/internal/services/action_values.go @@ -63,13 +63,7 @@ type ActionSecrets struct { Scope ActionScope } -// List returns secret metadata only. -func (s ActionSecrets) List(ctx context.Context, limit, page int) ([]ActionSecret, error) { - window, err := s.ListPage(ctx, limit, page) - return window.Items, err -} - -// ListPage is List with what the server reports about the other pages. +// ListPage lists a page of secrets, with what the server reports about the other pages. func (s ActionSecrets) ListPage(ctx context.Context, limit, page int) (Page[ActionSecret], error) { if s.Scope.Account { return Page[ActionSecret]{}, &CapabilityError{Feature: "account secret listing", Reason: "Gitea exposes account secret set/delete operations only"} @@ -88,7 +82,7 @@ func (s ActionSecrets) ListPage(ctx context.Context, limit, page int) (Page[Acti }) return secrets, headerOf(resp), err }) - return Page[ActionSecret]{Items: convertAll(window.Items, secretFrom), More: window.More, Total: window.Total, Counted: window.Counted}, err + return convertPage(window, secretFrom), err } // Set creates or replaces a secret; the API's PUT operation defines upsert semantics. The result @@ -147,13 +141,7 @@ func (s ActionVariables) repoVariables(name string) string { return endpoint } -// List returns a non-nil page of variables. -func (s ActionVariables) List(ctx context.Context, limit, page int) ([]ActionVariable, error) { - window, err := s.ListPage(ctx, limit, page) - return window.Items, err -} - -// ListPage is List with what the server reports about the other pages. +// ListPage lists a page of variables, with what the server reports about the other pages. func (s ActionVariables) ListPage(ctx context.Context, limit, page int) (Page[ActionVariable], error) { var window Page[*gitea.ActionsVariable] var err error @@ -170,7 +158,7 @@ func (s ActionVariables) ListPage(ctx context.Context, limit, page int) (Page[Ac } else { window, err = rawPage[*gitea.ActionsVariable](ctx, s.Client, s.repoVariables(""), nil, limit, page) } - return Page[ActionVariable]{Items: convertAll(window.Items, variableFrom), More: window.More, Total: window.Total, Counted: window.Counted}, err + return convertPage(window, variableFrom), err } // View retrieves one variable. diff --git a/internal/services/action_values_test.go b/internal/services/action_values_test.go index 9c7034ff..53235354 100644 --- a/internal/services/action_values_test.go +++ b/internal/services/action_values_test.go @@ -82,9 +82,9 @@ func TestVariableReadsAndDeletesUseTheirScope(t *testing.T) { })) service := ActionVariables{Client: client, Scope: scope.scope} want := ActionVariable{Name: "MODE", Value: "test", Description: "build mode"} - items, err := service.List(context.Background(), 20, 2) + items, err := service.ListPage(context.Background(), 20, 2) require.NoError(t, err) - require.Equal(t, []ActionVariable{want}, items) + require.Equal(t, []ActionVariable{want}, items.Items) item, err := service.View(context.Background(), "MODE") require.NoError(t, err) require.Equal(t, want, item) @@ -128,14 +128,14 @@ func TestSecretListReturnsMetadataOnly(t *testing.T) { assert.Equal(t, "2", r.URL.Query().Get("page")) _, _ = w.Write([]byte(`[{"name":"TOKEN","data":"never-display","description":"deploy","created_at":"2028-01-02T03:04:05Z"},{"name":"OTHER"}]`)) })) - items, err := (ActionSecrets{Client: client, Scope: scope.scope}).List(context.Background(), 20, 2) + items, err := (ActionSecrets{Client: client, Scope: scope.scope}).ListPage(context.Background(), 20, 2) if scope.scope.Account { var unsupported *CapabilityError require.ErrorAs(t, err, &unsupported) return } require.NoError(t, err) - require.Equal(t, []ActionSecret{{Name: "TOKEN", Description: "deploy", CreatedAt: "2028-01-02T03:04:05Z"}, {Name: "OTHER"}}, items) + require.Equal(t, []ActionSecret{{Name: "TOKEN", Description: "deploy", CreatedAt: "2028-01-02T03:04:05Z"}, {Name: "OTHER"}}, items.Items) }) } } diff --git a/internal/services/branches.go b/internal/services/branches.go index 4b53a5f6..a45a5c88 100644 --- a/internal/services/branches.go +++ b/internal/services/branches.go @@ -39,13 +39,7 @@ func branchFrom(branch *gitea.Branch) Branch { // Branches operates on remote branches and protection rules. type Branches struct{ Client *transport.Client } -// List returns a page of remote branches. -func (s Branches) List(ctx context.Context, repo string, limit, page int) ([]Branch, error) { - window, err := s.ListPage(ctx, repo, limit, page) - return window.Items, err -} - -// ListPage is List that also reports whether more branches follow and how many there are. +// ListPage lists a page of branches, with whether more follow and how many there are. func (s Branches) ListPage(ctx context.Context, repo string, limit, page int) (Page[Branch], error) { window, err := listPage(limit, page, func(limit, page int) ([]*gitea.Branch, http.Header, error) { var headers http.Header @@ -59,7 +53,7 @@ func (s Branches) ListPage(ctx context.Context, repo string, limit, page int) (P if err != nil { return Page[Branch]{}, err } - return Page[Branch]{Items: convertAll(window.Items, branchFrom), More: window.More, Total: window.Total, Counted: window.Counted}, nil + return convertPage(window, branchFrom), nil } // View retrieves a remote branch. @@ -143,7 +137,7 @@ func (s Branches) Unprotect(ctx context.Context, repo, rule string) (bool, error err := repoDo(ctx, s.Client, repo, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.Response, error) { return c.Repositories.DeleteBranchProtection(ctx, owner, name, rule) }) - if apiErr, ok := errors.AsType[*transport.APIError](err); ok && apiErr.Status == http.StatusNotFound { + if IsNotFound(err) { if _, err := (Repositories{Client: s.Client}).get(ctx, repo); err != nil { return false, err } diff --git a/internal/services/branches_test.go b/internal/services/branches_test.go index 997b0627..3937b212 100644 --- a/internal/services/branches_test.go +++ b/internal/services/branches_test.go @@ -36,12 +36,12 @@ func TestBranchListMapsServerFields(t *testing.T) { assert.Equal(t, "3", r.URL.Query().Get("page")) _, _ = w.Write([]byte(`[{"name":"main","commit":{"id":"abc","message":"Fix\n\nbody","timestamp":"2026-10-01T12:00:00Z"},"protected":true,"required_approvals":2,"user_can_push":true,"user_can_merge":true},{"name":"orphan","commit":null}]`)) })) - items, err := (Branches{Client: client}).List(context.Background(), "owner/repo", 20, 3) + items, err := (Branches{Client: client}).ListPage(context.Background(), "owner/repo", 20, 3) require.NoError(t, err) require.Equal(t, []Branch{ {Name: "main", SHA: "abc", Message: "Fix\n\nbody", UpdatedAt: "2026-10-01T12:00:00Z", Protected: true, RequiredApprovals: 2, CanPush: true, CanMerge: true}, {Name: "orphan"}, - }, items) + }, items.Items) } func TestBranchCreateAndProtectPayloads(t *testing.T) { diff --git a/internal/services/comments.go b/internal/services/comments.go index b165bdf8..c3e7701a 100644 --- a/internal/services/comments.go +++ b/internal/services/comments.go @@ -5,7 +5,6 @@ package services import ( "context" - "path" "strconv" gitea "gitea.dev/sdk" @@ -30,19 +29,10 @@ func commentFrom(comment *gitea.Comment) Comment { return Comment{ ID: comment.ID, Body: comment.Body, URL: comment.HTMLURL, Author: login(comment.Poster), CreatedAt: timestamp(comment.Created), UpdatedAt: timestamp(comment.Updated), Pull: comment.PRURL != "", - IssueNumber: discussionNumber(comment.IssueURL + comment.PRURL), + IssueNumber: lastPathNumber(comment.IssueURL + comment.PRURL), } } -// discussionNumber reads the number from the web URL of an issue or pull request, or returns 0. -func discussionNumber(url string) int64 { - n, err := strconv.ParseInt(path.Base(url), 10, 64) - if err != nil { - return 0 - } - return n -} - // Comments implements discussion comments under their owning repository resource. type Comments struct { Client *transport.Client diff --git a/internal/services/convert.go b/internal/services/convert.go index 29c476c2..dfe2afde 100644 --- a/internal/services/convert.go +++ b/internal/services/convert.go @@ -66,3 +66,8 @@ func issueFrom(issue *gitea.Issue) Issue { Assignees: convertAll(issue.Assignees, login), Milestone: issueMilestoneFrom(issue.Milestone), Comments: int64(issue.Comments), } } + +// convertPage maps a page of SDK items onto tea results. +func convertPage[S, T any](page Page[*S], convert func(*S) T) Page[T] { + return Page[T]{Items: convertAll(page.Items, convert), More: page.More, Total: page.Total, Counted: page.Counted} +} diff --git a/internal/services/errors.go b/internal/services/errors.go index 413775bd..a0589489 100644 --- a/internal/services/errors.go +++ b/internal/services/errors.go @@ -194,6 +194,12 @@ func Classify(err error, kind, key, scope string) error { return err } +// IsNotFound reports whether err is a 404 response. +func IsNotFound(err error) bool { + failure, ok := errors.AsType[*transport.APIError](err) + return ok && failure.Status == http.StatusNotFound +} + // refusal returns the conflict reason of a refused request, or "". func refusal(apiErr *transport.APIError) string { if apiErr.Status == http.StatusLocked { diff --git a/internal/services/issue_dependencies.go b/internal/services/issue_dependencies.go index 5e3f292e..aae12604 100644 --- a/internal/services/issue_dependencies.go +++ b/internal/services/issue_dependencies.go @@ -5,11 +5,8 @@ package services import ( "context" - "errors" - "net/http" gitea "gitea.dev/sdk" - "gitea.dev/tea/internal/transport" ) // IssueReference identifies an issue that may live in another repository. @@ -57,7 +54,7 @@ func (s Issues) Dependencies(ctx context.Context, number int64) (IssueDependenci }) return convertAll(issues, issueReferenceFrom), err }) - if api, ok := errors.AsType[*transport.APIError](err); ok && api.Status == http.StatusNotFound { + if IsNotFound(err) { return IssueDependencies{}, nil } if err != nil { diff --git a/internal/services/issues.go b/internal/services/issues.go index 5f1e8d23..f9bc5cda 100644 --- a/internal/services/issues.go +++ b/internal/services/issues.go @@ -132,7 +132,7 @@ func (s Issues) ListPage(ctx context.Context, opts ListOptions) (Page[Issue], er if err != nil { return Page[Issue]{}, err } - return Page[Issue]{Items: convertAll(issues.Items, discussionFrom), More: issues.More, Total: issues.Total, Counted: issues.Counted}, nil + return convertPage(issues, discussionFrom), nil } // filterValues splits a comma-separated filter, which the SDK joins again. diff --git a/internal/services/metadata.go b/internal/services/metadata.go index a130b39b..4a376a95 100644 --- a/internal/services/metadata.go +++ b/internal/services/metadata.go @@ -58,7 +58,7 @@ func (s Labels) ListPage(ctx context.Context, limit, page int) (Page[Label], err labels, resp, err := s.list(ctx, limit, page) return labels, headerOf(resp), err }) - return Page[Label]{Items: convertAll(labels.Items, labelFrom), More: labels.More, Total: labels.Total, Counted: labels.Counted}, err + return convertPage(labels, labelFrom), err } func (s Labels) list(ctx context.Context, limit, page int) ([]*gitea.Label, *gitea.Response, error) { @@ -218,7 +218,7 @@ func (s Milestones) ListPage(ctx context.Context, state string, limit, page int) }) return milestones, headerOf(resp), err }) - return Page[Milestone]{Items: convertAll(milestones.Items, milestoneFrom), More: milestones.More, Total: milestones.Total, Counted: milestones.Counted}, err + return convertPage(milestones, milestoneFrom), err } // Create creates one milestone. diff --git a/internal/services/milestone_selection.go b/internal/services/milestone_selection.go index f81ad4fa..4b315d39 100644 --- a/internal/services/milestone_selection.go +++ b/internal/services/milestone_selection.go @@ -5,36 +5,9 @@ package services import ( "context" - "fmt" "strconv" - "strings" ) -// ResolveName selects an exact title across open and closed milestones. -func (s Milestones) ResolveName(ctx context.Context, name string) (int64, error) { - if strings.TrimSpace(name) == "" { - return 0, fmt.Errorf("milestone name cannot be empty") - } - items, err := s.All(ctx) - if err != nil { - return 0, err - } - var id int64 - for _, item := range items { - if item.Title != name { - continue - } - if id != 0 { - return 0, fmt.Errorf("milestone name %q is ambiguous; select its numeric ID", name) - } - id = item.ID - } - if id == 0 { - return 0, fmt.Errorf("milestone %q does not exist; use tea milestone list --state all", name) - } - return id, nil -} - // All returns the open and closed milestones of the repository. func (s Milestones) All(ctx context.Context) ([]Milestone, error) { return listAll(func(page int) ([]Milestone, error) { return s.List(ctx, "all", 100, page) }) diff --git a/internal/services/milestone_selection_test.go b/internal/services/milestone_selection_test.go index f85a5b58..38cf3c4d 100644 --- a/internal/services/milestone_selection_test.go +++ b/internal/services/milestone_selection_test.go @@ -32,12 +32,12 @@ func TestMilestoneNamesIncludeClosedAndRejectAmbiguity(t *testing.T) { } })) service := Milestones{Client: client, Repository: "alice/project"} - id, err := service.ResolveName(context.Background(), "Release") + item, err := service.FindOne(context.Background(), "Release", false) require.NoError(t, err) - require.EqualValues(t, 2, id) - _, err = service.ResolveName(context.Background(), "missing") - require.ErrorContains(t, err, "does not exist") + require.EqualValues(t, 2, item.ID) + _, err = service.FindOne(context.Background(), "missing", false) + require.ErrorContains(t, err, "not found") duplicate = true - _, err = service.ResolveName(context.Background(), "Release") + _, err = service.FindOne(context.Background(), "Release", false) require.ErrorContains(t, err, "ambiguous") } diff --git a/internal/services/organizations.go b/internal/services/organizations.go index 7432ced6..36caecde 100644 --- a/internal/services/organizations.go +++ b/internal/services/organizations.go @@ -34,7 +34,7 @@ func organization(ctx context.Context, client *transport.Client, owner string) ( org, _, err := call(ctx, client, func(ctx context.Context, c *gitea.Client) (*gitea.Organization, *gitea.Response, error) { return c.Organizations.GetOrg(ctx, owner) }) - if statusNotFound(err) { + if IsNotFound(err) { return 0, false, nil } if err != nil { @@ -46,12 +46,6 @@ func organization(ctx context.Context, client *transport.Client, owner string) ( return org.ID, true, nil } -// statusNotFound reports whether err is a 404 response. -func statusNotFound(err error) bool { - failure, ok := errors.AsType[*transport.APIError](err) - return ok && failure.Status == http.StatusNotFound -} - // OrganizationOptions permits explicit empty and false preference updates. type OrganizationOptions struct { Name *string `json:"username,omitempty"` @@ -84,11 +78,6 @@ func (s Organizations) List(ctx context.Context, mine bool, username string, lim return convertPage(orgs, organizationFrom), err } -// convertPage maps a page of SDK items onto tea results. -func convertPage[S, T any](page Page[*S], convert func(*S) T) Page[T] { - return Page[T]{Items: convertAll(page.Items, convert), More: page.More, Total: page.Total, Counted: page.Counted} -} - // View retrieves a named organization. func (s Organizations) View(ctx context.Context, name string) (Organization, error) { org, _, err := call(ctx, s.Client, func(ctx context.Context, c *gitea.Client) (*gitea.Organization, *gitea.Response, error) { @@ -285,7 +274,7 @@ func (s Organizations) IsPublicMember(ctx context.Context, name, username string // check runs a membership check that answers no with a 404. func (s Organizations) check(ctx context.Context, op func(context.Context, *gitea.Client) (*gitea.Response, error)) (bool, error) { err := do(ctx, s.Client, op) - if statusNotFound(err) { + if IsNotFound(err) { return false, nil } return err == nil, err diff --git a/internal/services/pulls.go b/internal/services/pulls.go index 5c676c32..00a7cfe1 100644 --- a/internal/services/pulls.go +++ b/internal/services/pulls.go @@ -224,7 +224,7 @@ func (s Pulls) listPage(ctx context.Context, options PullListOptions) (Page[Pull if err != nil { return Page[Pull]{}, err } - return Page[Pull]{Items: convertAll(page.Items, pullFrom), More: page.More, Total: page.Total, Counted: page.Counted}, nil + return convertPage(page, pullFrom), nil } // View retrieves one pull request. @@ -476,7 +476,7 @@ func (s Pulls) Template(ctx context.Context, ref string) (string, error) { return c.Repositories.GetFileReader(ctx, owner, name, ref, path) }) if err != nil { - if apiError, ok := errors.AsType[*transport.APIError](err); ok && apiError.Status == http.StatusNotFound { + if IsNotFound(err) { continue } return "", err diff --git a/internal/services/releases.go b/internal/services/releases.go index f4dc892c..ef828db3 100644 --- a/internal/services/releases.go +++ b/internal/services/releases.go @@ -55,13 +55,7 @@ type Releases struct { func (s Releases) endpoint() string { return "repos/" + s.Repository + "/releases" } -// List returns a non-nil page of releases. -func (s Releases) List(ctx context.Context, limit, page int) ([]Release, error) { - window, err := s.ListPage(ctx, limit, page) - return window.Items, err -} - -// ListPage is List that also reports whether more releases follow and how many there are. +// ListPage lists a page of releases, with whether more follow and how many there are. func (s Releases) ListPage(ctx context.Context, limit, page int) (Page[Release], error) { window, err := listPage(limit, page, func(limit, page int) ([]*gitea.Release, http.Header, error) { var headers http.Header @@ -75,7 +69,7 @@ func (s Releases) ListPage(ctx context.Context, limit, page int) (Page[Release], if err != nil { return Page[Release]{}, err } - return Page[Release]{Items: convertAll(window.Items, releaseFrom), More: window.More, Total: window.Total, Counted: window.Counted}, nil + return convertPage(window, releaseFrom), nil } // View retrieves a release by its tag, including numeric tags. diff --git a/internal/services/releases_test.go b/internal/services/releases_test.go index 6fb45c9d..4a3bb950 100644 --- a/internal/services/releases_test.go +++ b/internal/services/releases_test.go @@ -171,9 +171,9 @@ func TestReleaseListAndViewEscapeTags(t *testing.T) { _, _ = w.Write([]byte(`{"id":2,"tag_name":"v1/beta one"}`)) })) service := Releases{Client: client, Repository: "alice/project"} - items, err := service.List(context.Background(), 10, 2) + items, err := service.ListPage(context.Background(), 10, 2) require.NoError(t, err) - require.Equal(t, []Release{{ID: 1, Tag: "v1", Assets: []ReleaseAsset{}}}, items) + require.Equal(t, []Release{{ID: 1, Tag: "v1", Assets: []ReleaseAsset{}}}, items.Items) item, err := service.View(context.Background(), "v1/beta one") require.NoError(t, err) require.Equal(t, "v1/beta one", item.Tag) diff --git a/internal/services/repositories.go b/internal/services/repositories.go index 6302a9e8..0f3bf3b4 100644 --- a/internal/services/repositories.go +++ b/internal/services/repositories.go @@ -109,7 +109,7 @@ func (s Repositories) Readme(ctx context.Context, repo, ref string) (name, conte return c.Repositories.ListContents(ctx, owner, name, ref, "") }) if err != nil { - if apiErr, ok := errors.AsType[*transport.APIError](err); ok && apiErr.Status == http.StatusNotFound { + if IsNotFound(err) { return "", "", nil } return "", "", err diff --git a/internal/services/repository_search.go b/internal/services/repository_search.go index 138b763f..4d485638 100644 --- a/internal/services/repository_search.go +++ b/internal/services/repository_search.go @@ -136,5 +136,5 @@ func (s Repositories) Search(ctx context.Context, opts RepositorySearchOptions) if err != nil { return Page[Repository]{}, err } - return Page[Repository]{Items: convertAll(window.Items, repositoryFrom), More: window.More, Total: window.Total, Counted: window.Counted}, nil + return convertPage(window, repositoryFrom), nil } diff --git a/internal/services/wiki.go b/internal/services/wiki.go index d63c59ec..3e1a2557 100644 --- a/internal/services/wiki.go +++ b/internal/services/wiki.go @@ -6,9 +6,7 @@ package services import ( "context" "encoding/base64" - "errors" "fmt" - "net/http" "strconv" gitea "gitea.dev/sdk" @@ -81,17 +79,11 @@ func (s Wiki) page(ctx context.Context, op repoOperation[*gitea.WikiPage]) (Wiki return wikiContentFrom(page) } -// List returns a non-nil page of wiki metadata. -func (s Wiki) List(ctx context.Context, limit, page int) ([]WikiPage, error) { - window, err := s.ListPage(ctx, limit, page) - return window.Items, err -} - // wikiPageSize is the page size for reading the whole wiki; Gitea replaces a limit of 1 with its // default page size. const wikiPageSize = 50 -// ListPage is List that also reports whether more pages follow and how many there are. It reads +// ListPage lists a page of wiki pages, with whether more follow and how many there are. It reads // the whole page list and pages it on the client, because the server pages it inconsistently. func (s Wiki) ListPage(ctx context.Context, limit, page int) (Page[WikiPage], error) { all, err := listAll(func(page int) ([]*gitea.WikiPageMetaData, error) { @@ -100,7 +92,7 @@ func (s Wiki) ListPage(ctx context.Context, limit, page int) (Page[WikiPage], er }) }) if err != nil { - if apiError, ok := errors.AsType[*transport.APIError](err); !ok || apiError.Status != http.StatusNotFound { + if !IsNotFound(err) { return Page[WikiPage]{}, err } // An uninitialized wiki returns 404. Verify repository visibility and diff --git a/internal/services/wiki_test.go b/internal/services/wiki_test.go index c7df529c..af7cb2d7 100644 --- a/internal/services/wiki_test.go +++ b/internal/services/wiki_test.go @@ -59,14 +59,14 @@ func TestWikiEmptyListDoesNotHideRepositoryErrors(t *testing.T) { } w.WriteHeader(http.StatusNotFound) })) - items, err := (Wiki{Client: client, Repository: "alice/project"}).List(context.Background(), 30, 1) + items, err := (Wiki{Client: client, Repository: "alice/project"}).ListPage(context.Background(), 30, 1) if !wikiEnabled { require.Error(t, err) return } require.NoError(t, err) - require.NotNil(t, items) - require.Empty(t, items) + require.NotNil(t, items.Items) + require.Empty(t, items.Items) }) } } @@ -95,9 +95,9 @@ func TestWikiListAndRevisionsMapCommits(t *testing.T) { {Title: "Other"}, {Title: "Home", URL: "https://gitea.test/wiki/Home", SHA: "abc", Author: "Alice", UpdatedAt: "2026-10-04T00:00:00Z"}, }, More: true, Total: 3, Counted: true}, pages) - last, err := service.List(context.Background(), 2, 2) + last, err := service.ListPage(context.Background(), 2, 2) require.NoError(t, err) - require.Equal(t, []WikiPage{{Title: "Draft"}}, last) + require.Equal(t, []WikiPage{{Title: "Draft"}}, last.Items) revisions, err := service.Revisions(context.Background(), "Home", 3) require.NoError(t, err) require.Equal(t, []WikiRevision{{SHA: "abc", Message: "Edit", Author: "Alice", CreatedAt: "2026-10-04T00:00:00Z"}, {SHA: "def"}}, revisions) diff --git a/internal/terminal/table.go b/internal/terminal/table.go index 1f27413f..a71d78a3 100644 --- a/internal/terminal/table.go +++ b/internal/terminal/table.go @@ -83,6 +83,9 @@ func (c Cell) style(text string, d Display, link bool) string { return Hyperlink(c.Link, text, link && d.Links) } +// Render styles the cell's text outside a table: its color, muted or bold, without a link. +func (c Cell) Render(d Display) string { return c.style(SafeLine(c.Text), d, false) } + // Table renders rows under columns; rows may have more cells than there are columns. On a TTY it // prints an uppercase header row when any column has a header, aligns columns two spaces apart, // and shrinks Flex columns until rows fit the display width. Otherwise it writes one line of