diff --git a/internal/app/organizations.go b/internal/app/organizations.go index 397b5713..d55e0175 100644 --- a/internal/app/organizations.go +++ b/internal/app/organizations.go @@ -292,11 +292,11 @@ $ tea org delete acme-old --yes`, if err != nil { return err } - size, err := service.Size(cmd.Context(), org.Name) + repos, err := service.RepositoryCount(cmd.Context(), org.Name) if err != nil { return err } - if size.Repositories != nil && *size.Repositories > 0 { + if repos != nil && *repos > 0 { return &services.ConflictError{Reason: services.ReasonOwnsRepos, Kind: "organization", Key: org.Name, Hint: "transfer or delete them first (see tea repo list " + org.Name + ")"} } if err := a.confirmAction(cmd, terminal.Confirmation{ diff --git a/internal/app/pull_mutations.go b/internal/app/pull_mutations.go index 9a4b8312..2bd33547 100644 --- a/internal/app/pull_mutations.go +++ b/internal/app/pull_mutations.go @@ -176,11 +176,7 @@ tea pr diff 13 | git apply works.`, Annotations: pullArgumentsAnnotation(textOut } service := services.Pulls{Client: t.client, Repository: t.Repo} if web { - item, err := service.View(cmd.Context(), n) - if err != nil { - return err - } - return a.openBrowser(cmd, item.URL+"/files") + return a.openBrowser(cmd, fmt.Sprintf("%s/pulls/%d/files", repositoryURL(t), n)) } if nameOnly { files, err := service.Files(cmd.Context(), n) diff --git a/internal/app/pulls.go b/internal/app/pulls.go index 115c047e..0a0990c3 100644 --- a/internal/app/pulls.go +++ b/internal/app/pulls.go @@ -78,10 +78,8 @@ each pull request, which tea only makes when --json names them.`, Args: argsExac } items := window.Items if a.jsonSelects("reviewers", "reviewer_teams", "merged_by") { - for i, item := range items { - if items[i], err = service.View(cmd.Context(), item.Number); err != nil { - return err - } + if items, err = service.ViewAll(cmd.Context(), items); err != nil { + return err } } noun := map[string]string{"open": "open pull requests", "closed": "closed pull requests", "merged": "merged pull requests", "all": "pull requests"}[state] diff --git a/internal/services/organizations.go b/internal/services/organizations.go index 36caecde..794e498b 100644 --- a/internal/services/organizations.go +++ b/internal/services/organizations.go @@ -94,7 +94,8 @@ type OrganizationSize struct { Members, Teams, Repositories *int } -// Size counts the members, teams and repositories of an organization. +// Size counts the members, teams and repositories of an organization; a count the account cannot +// read is nil. func (s Organizations) Size(ctx context.Context, name string) (OrganizationSize, error) { first := listOptions(1, 1) counts := []func(context.Context, *gitea.Client) (*gitea.Response, error){ @@ -106,24 +107,40 @@ func (s Organizations) Size(ctx context.Context, name string) (OrganizationSize, _, resp, err := c.Organizations.ListOrgTeams(ctx, name, gitea.ListTeamsOptions{ListOptions: first}) return resp, err }, - func(ctx context.Context, c *gitea.Client) (*gitea.Response, error) { - _, resp, err := c.Repositories.ListOrgRepos(ctx, name, gitea.ListOrgReposOptions{ListOptions: first}) - return resp, err - }, + orgRepos(name), } totals := make([]*int, len(counts)) - for i, count := range counts { - total, err := listTotal(ctx, s.Client, count) - if ctx.Err() != nil { - return OrganizationSize{}, ctx.Err() - } - if err == nil { - totals[i] = total - } + _ = concurrently(len(counts), func(i int) error { + totals[i], _ = listTotal(ctx, s.Client, counts[i]) + return nil + }) + if ctx.Err() != nil { + return OrganizationSize{}, ctx.Err() } return OrganizationSize{Members: totals[0], Teams: totals[1], Repositories: totals[2]}, nil } +// RepositoryCount counts the repositories of an organization, or returns nil when the account +// cannot read the count. +func (s Organizations) RepositoryCount(ctx context.Context, name string) (*int, error) { + total, err := listTotal(ctx, s.Client, orgRepos(name)) + if ctx.Err() != nil { + return nil, ctx.Err() + } + if err != nil { + return nil, nil + } + return total, nil +} + +// orgRepos requests the first repository of an organization, to count them. +func orgRepos(name string) func(context.Context, *gitea.Client) (*gitea.Response, error) { + return func(ctx context.Context, c *gitea.Client) (*gitea.Response, error) { + _, resp, err := c.Repositories.ListOrgRepos(ctx, name, gitea.ListOrgReposOptions{ListOptions: listOptions(1, 1)}) + return resp, err + } +} + // listTotal returns the collection size a list request reports, or nil when the server does not count it. func listTotal(ctx context.Context, client *transport.Client, list func(context.Context, *gitea.Client) (*gitea.Response, error)) (*int, error) { _, resp, err := call(ctx, client, func(ctx context.Context, c *gitea.Client) (struct{}, *gitea.Response, error) { diff --git a/internal/services/pulls.go b/internal/services/pulls.go index 00a7cfe1..3469caab 100644 --- a/internal/services/pulls.go +++ b/internal/services/pulls.go @@ -236,6 +236,17 @@ func (s Pulls) View(ctx context.Context, n int64) (Pull, error) { return pullFrom(pull), nil } +// ViewAll retrieves each of items again, a few at a time, for the fields only a single pull +// request carries, such as its reviewers. +func (s Pulls) ViewAll(ctx context.Context, items []Pull) ([]Pull, error) { + viewed := make([]Pull, len(items)) + err := concurrently(len(items), func(i int) (err error) { + viewed[i], err = s.View(ctx, items[i].Number) + return err + }) + return viewed, err +} + func (s Pulls) get(ctx context.Context, n int64) (*gitea.PullRequest, error) { item, err := repoCall(ctx, s.Client, s.Repository, func(ctx context.Context, c *gitea.Client, owner, name string) (*gitea.PullRequest, *gitea.Response, error) { return c.PullRequests.GetPullRequest(ctx, owner, name, n) @@ -264,15 +275,15 @@ func (s Pulls) Create(ctx context.Context, opts PullOptions) (Pull, error) { // Edit updates only explicitly supplied resource fields. func (s Pulls) Edit(ctx context.Context, n int64, opts PullEdit) (Pull, error) { if opts.ReviewerChanges != nil { - item, err := (Reviews{Client: s.Client, Repository: s.Repository}).ChangeRequests(ctx, n, *opts.ReviewerChanges) - if err != nil { - return Pull{}, err - } + reviews, changes := Reviews{Client: s.Client, Repository: s.Repository}, opts.ReviewerChanges opts.ReviewerChanges = nil if opts == (PullEdit{}) { - return item, nil + return reviews.ChangeRequests(ctx, n, *changes) } - item, err = s.Edit(ctx, n, opts) + if err := reviews.changeRequests(ctx, n, *changes); err != nil { + return Pull{}, err + } + item, err := s.Edit(ctx, n, opts) if err != nil { return Pull{}, fmt.Errorf("review requests were updated, but editing other pull request fields failed: %w", err) } diff --git a/internal/services/review_requests.go b/internal/services/review_requests.go index b530549c..dd654bcb 100644 --- a/internal/services/review_requests.go +++ b/internal/services/review_requests.go @@ -51,12 +51,24 @@ func (c ReviewRequestChanges) Validate() error { // ChangeRequests returns the PR after changes and reports partial mutation failures. func (s Reviews) ChangeRequests(ctx context.Context, n int64, changes ReviewRequestChanges) (Pull, error) { - if err := changes.Validate(); err != nil { + if err := s.changeRequests(ctx, n, changes); err != nil { return Pull{}, err } + item, err := (Pulls{Client: s.Client, Repository: s.Repository}).View(ctx, n) + if err != nil { + return Pull{}, fmt.Errorf("review requests were updated, but reading the pull request failed: %w", err) + } + return item, nil +} + +// changeRequests applies the review request changes to pull request n. +func (s Reviews) changeRequests(ctx context.Context, n int64, changes ReviewRequestChanges) error { + if err := changes.Validate(); err != nil { + return err + } if len(changes.Add.Teams)+len(changes.Remove.Teams) > 0 { if err := s.CheckRequestTeams(ctx, append(append([]string{}, changes.Add.Teams...), changes.Remove.Teams...)); err != nil { - return Pull{}, err + return err } } mutated := false @@ -76,17 +88,13 @@ func (s Reviews) ChangeRequests(ctx context.Context, n int64, changes ReviewRequ }) if err != nil { if mutated { - return Pull{}, fmt.Errorf("reviewer removals succeeded, but adding reviewers failed: %w", err) + return fmt.Errorf("reviewer removals succeeded, but adding reviewers failed: %w", err) } - return Pull{}, fmt.Errorf("review request update failed; Gitea may have applied some requests: %w", err) + return fmt.Errorf("review request update failed; Gitea may have applied some requests: %w", err) } mutated = true } - item, err := (Pulls{Client: s.Client, Repository: s.Repository}).View(ctx, n) - if err != nil { - return Pull{}, fmt.Errorf("review requests were updated, but reading the pull request failed: %w", err) - } - return item, nil + return nil } // CheckRequestTeams verifies that every named team exists in the repository's organization. diff --git a/internal/services/teams.go b/internal/services/teams.go index a4ab9656..63500be6 100644 --- a/internal/services/teams.go +++ b/internal/services/teams.go @@ -203,7 +203,8 @@ func (s Teams) Sizes(ctx context.Context, teams []Team) ([]TeamSize, error) { _, resp, err := c.Organizations.ListTeamMembers(ctx, id, gitea.ListTeamMembersOptions{ListOptions: first}) return resp, err }) - if err != nil { + if err != nil || teams[i].AllRepositories { + sizes[i] = TeamSize{Members: members} return err } repositories, err := listTotal(ctx, s.Client, func(ctx context.Context, c *gitea.Client) (*gitea.Response, error) { diff --git a/internal/terminal/markdown.go b/internal/terminal/markdown.go index 0ed8024f..a062a87f 100644 --- a/internal/terminal/markdown.go +++ b/internal/terminal/markdown.go @@ -6,6 +6,7 @@ package terminal import ( "regexp" "strings" + "sync" "charm.land/glamour/v2" "charm.land/glamour/v2/styles" @@ -19,6 +20,34 @@ func MarkdownEnabled(preference string, outTTY bool) bool { // glamourLinks matches the OSC 8 hyperlinks that glamour writes for markdown links. var glamourLinks = regexp.MustCompile("\x1b\\]8;[^\x07\x1b]*(?:\x07|\x1b\\\\)") +// renderers caches a glamour renderer per width and color, as building one parses its style each +// time; the lock also serializes rendering. +var renderers = struct { + sync.Mutex + byKey map[[2]int]*glamour.TermRenderer +}{byKey: map[[2]int]*glamour.TermRenderer{}} + +func markdownRenderer(width int, color bool) (*glamour.TermRenderer, error) { + key := [2]int{width, 0} + if color { + key[1] = 1 + } + if renderer := renderers.byKey[key]; renderer != nil { + return renderer, nil + } + style := styles.NoTTYStyleConfig + if color { + style = styles.DarkStyleConfig + // The terminal's own text color keeps body text readable on light backgrounds too. + style.Document.Color = nil + } + renderer, err := glamour.NewTermRenderer(glamour.WithStyles(style), glamour.WithWordWrap(width), glamour.WithPreservedNewLines()) + if err == nil { + renderers.byKey[key] = renderer + } + return renderer, err +} + // Markdown renders sanitized markdown for d: wrapped to its width (80 when unknown), colored or plain, // with links only when d allows them. It keeps line breaks like Gitea does, and returns the sanitized // text unrendered if rendering fails. @@ -28,17 +57,14 @@ func Markdown(text string, d Display) string { if width <= 0 { width = 80 } - style := styles.NoTTYStyleConfig - if d.Color { - style = styles.DarkStyleConfig - // The terminal's own text color keeps body text readable on light backgrounds too. - style.Document.Color = nil - } - renderer, err := glamour.NewTermRenderer(glamour.WithStyles(style), glamour.WithWordWrap(width), glamour.WithPreservedNewLines()) + renderers.Lock() + renderer, err := markdownRenderer(width, d.Color) if err != nil { + renderers.Unlock() return text } rendered, err := renderer.Render(text) + renderers.Unlock() if err != nil { return text }