diff --git a/cmd/admin/users/list.go b/cmd/admin/users/list.go index 3a7642b..cc831d1 100644 --- a/cmd/admin/users/list.go +++ b/cmd/admin/users/list.go @@ -43,7 +43,7 @@ func RunUserList(_ stdctx.Context, cmd *cli.Command) error { client := ctx.Login.Client() users, _, err := client.AdminListUsers(gitea.AdminListUsersOptions{ - ListOptions: ctx.GetListOptions(), + ListOptions: flags.GetListOptions(), }) if err != nil { return err diff --git a/cmd/attachments/list.go b/cmd/attachments/list.go index 0b11224..be6c142 100644 --- a/cmd/attachments/list.go +++ b/cmd/attachments/list.go @@ -46,7 +46,7 @@ func RunReleaseAttachmentList(_ stdctx.Context, cmd *cli.Command) error { } attachments, _, err := ctx.Login.Client().ListReleaseAttachments(ctx.Owner, ctx.Repo, release.ID, gitea.ListReleaseAttachmentsOptions{ - ListOptions: ctx.GetListOptions(), + ListOptions: flags.GetListOptions(), }) if err != nil { return err diff --git a/cmd/branches/list.go b/cmd/branches/list.go index 165ef6c..ce47330 100644 --- a/cmd/branches/list.go +++ b/cmd/branches/list.go @@ -50,7 +50,7 @@ func RunBranchesList(_ stdctx.Context, cmd *cli.Command) error { var protections []*gitea.BranchProtection var err error branches, _, err = ctx.Login.Client().ListRepoBranches(owner, ctx.Repo, gitea.ListRepoBranchesOptions{ - ListOptions: ctx.GetListOptions(), + ListOptions: flags.GetListOptions(), }) if err != nil { @@ -58,7 +58,7 @@ func RunBranchesList(_ stdctx.Context, cmd *cli.Command) error { } protections, _, err = ctx.Login.Client().ListBranchProtections(owner, ctx.Repo, gitea.ListBranchProtectionsOptions{ - ListOptions: ctx.GetListOptions(), + ListOptions: flags.GetListOptions(), }) if err != nil { diff --git a/cmd/flags/generic.go b/cmd/flags/generic.go index 6f9d2c9..f942a70 100644 --- a/cmd/flags/generic.go +++ b/cmd/flags/generic.go @@ -1,9 +1,12 @@ -// Copyright 2019 The Gitea Authors. All rights reserved. +// Copyright 2025 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT package flags import ( + "errors" + + "code.gitea.io/sdk/gitea" "github.com/urfave/cli/v3" ) @@ -35,12 +38,38 @@ var OutputFlag = cli.StringFlag{ Usage: "Output format. (simple, table, csv, tsv, yaml, json)", } +var ( + paging gitea.ListOptions + // ErrPage indicates that the provided page value is invalid (less than -1 or equal to 0). + ErrPage = errors.New("page cannot be smaller than 1") + // ErrLimit indicates that the provided limit value is invalid (negative). + ErrLimit = errors.New("limit cannot be negative") +) + +// GetListOptions returns configured paging struct +func GetListOptions() gitea.ListOptions { + return paging +} + +// PaginationFlags provides all pagination related flags +var PaginationFlags = []cli.Flag{ + &PaginationPageFlag, + &PaginationLimitFlag, +} + // PaginationPageFlag provides flag for pagination options var PaginationPageFlag = cli.IntFlag{ Name: "page", Aliases: []string{"p"}, Usage: "specify page", Value: 1, + Validator: func(i int) error { + if i < 1 && i != -1 { + return ErrPage + } + return nil + }, + Destination: &paging.Page, } // PaginationLimitFlag provides flag for pagination options @@ -49,6 +78,13 @@ var PaginationLimitFlag = cli.IntFlag{ Aliases: []string{"lm"}, Usage: "specify limit of items per page", Value: 30, + Validator: func(i int) error { + if i < 0 { + return ErrLimit + } + return nil + }, + Destination: &paging.PageSize, } // LoginOutputFlags defines login and output flags that should diff --git a/cmd/flags/generic_test.go b/cmd/flags/generic_test.go new file mode 100644 index 0000000..9f290b5 --- /dev/null +++ b/cmd/flags/generic_test.go @@ -0,0 +1,125 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package flags + +import ( + "context" + "io" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/urfave/cli/v3" +) + +func TestPaginationFlags(t *testing.T) { + var ( + defaultPage = PaginationPageFlag.Value + defaultLimit = PaginationLimitFlag.Value + ) + + cases := []struct { + name string + args []string + expectedPage int + expectedLimit int + }{ + { + name: "no flags", + args: []string{"test"}, + expectedPage: defaultPage, + expectedLimit: defaultLimit, + }, + { + name: "only paging", + args: []string{"test", "--page", "5"}, + expectedPage: 5, + expectedLimit: defaultLimit, + }, + { + name: "only limit", + args: []string{"test", "--limit", "10"}, + expectedPage: defaultPage, + expectedLimit: 10, + }, + { + name: "only limit", + args: []string{"test", "--limit", "10"}, + expectedPage: defaultPage, + expectedLimit: 10, + }, + { + name: "both flags", + args: []string{"test", "--page", "2", "--limit", "20"}, + expectedPage: 2, + expectedLimit: 20, + }, + { //TODO: Should no paging be applied as -1 or a separate flag? It's not obvious that page=-1 turns off paging and limit is ignored + name: "no paging", + args: []string{"test", "--limit", "20", "--page", "-1"}, + expectedPage: -1, + expectedLimit: 20, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cmd := cli.Command{ + Name: "test-paging", + Action: func(_ context.Context, cmd *cli.Command) error { + assert.Equal(t, tc.expectedPage, cmd.Int("page")) + assert.Equal(t, tc.expectedLimit, cmd.Int("limit")) + return nil + }, + Flags: PaginationFlags, + } + err := cmd.Run(context.Background(), tc.args) + require.NoError(t, err) + }) + } + +} +func TestPaginationFailures(t *testing.T) { + cases := []struct { + name string + args []string + expectedError error + }{ + { + name: "negative limit", + args: []string{"test", "--limit", "-10"}, + expectedError: ErrLimit, + }, + { + name: "negative paging", + args: []string{"test", "--page", "-2"}, + expectedError: ErrPage, + }, + { + name: "zero paging", + args: []string{"test", "--page", "0"}, + expectedError: ErrPage, + }, + { + //urfave does not validate all flags in one pass + name: "negative paging and paging", + args: []string{"test", "--page", "-2", "--limit", "-10"}, + expectedError: ErrPage, + }, + } + + for _, tc := range cases { + cmd := cli.Command{ + Name: "test-paging", + Flags: PaginationFlags, + Writer: io.Discard, + ErrWriter: io.Discard, + } + t.Run(tc.name, func(t *testing.T) { + err := cmd.Run(context.Background(), tc.args) + require.ErrorContains(t, err, tc.expectedError.Error()) + // require.ErrorIs(t, err, tc.expectedError) + }) + } +} diff --git a/cmd/issues/list.go b/cmd/issues/list.go index a3a9664..3bca2b4 100644 --- a/cmd/issues/list.go +++ b/cmd/issues/list.go @@ -85,7 +85,7 @@ func RunIssuesList(_ stdctx.Context, cmd *cli.Command) error { var issues []*gitea.Issue if ctx.Repo != "" { issues, _, err = ctx.Login.Client().ListRepoIssues(owner, ctx.Repo, gitea.ListIssueOption{ - ListOptions: ctx.GetListOptions(), + ListOptions: flags.GetListOptions(), State: state, Type: kind, KeyWord: ctx.String("keyword"), @@ -103,7 +103,7 @@ func RunIssuesList(_ stdctx.Context, cmd *cli.Command) error { } } else { issues, _, err = ctx.Login.Client().ListIssues(gitea.ListIssueOption{ - ListOptions: ctx.GetListOptions(), + ListOptions: flags.GetListOptions(), State: state, Type: kind, KeyWord: ctx.String("keyword"), diff --git a/cmd/labels/list.go b/cmd/labels/list.go index 889ba3d..fcbe90d 100644 --- a/cmd/labels/list.go +++ b/cmd/labels/list.go @@ -41,7 +41,7 @@ func RunLabelsList(_ stdctx.Context, cmd *cli.Command) error { client := ctx.Login.Client() labels, _, err := client.ListRepoLabels(ctx.Owner, ctx.Repo, gitea.ListLabelsOptions{ - ListOptions: ctx.GetListOptions(), + ListOptions: flags.GetListOptions(), }) if err != nil { return err diff --git a/cmd/milestones/issues.go b/cmd/milestones/issues.go index 6b7104a..7d056ba 100644 --- a/cmd/milestones/issues.go +++ b/cmd/milestones/issues.go @@ -103,7 +103,7 @@ func runMilestoneIssueList(_ stdctx.Context, cmd *cli.Command) error { } issues, _, err := client.ListRepoIssues(ctx.Owner, ctx.Repo, gitea.ListIssueOption{ - ListOptions: ctx.GetListOptions(), + ListOptions: flags.GetListOptions(), Milestones: []string{milestone}, Type: kind, State: state, diff --git a/cmd/milestones/list.go b/cmd/milestones/list.go index 2f30eb8..e229c56 100644 --- a/cmd/milestones/list.go +++ b/cmd/milestones/list.go @@ -61,7 +61,7 @@ func RunMilestonesList(_ stdctx.Context, cmd *cli.Command) error { client := ctx.Login.Client() milestones, _, err := client.ListRepoMilestones(ctx.Owner, ctx.Repo, gitea.ListMilestoneOption{ - ListOptions: ctx.GetListOptions(), + ListOptions: flags.GetListOptions(), State: state, }) diff --git a/cmd/notifications/list.go b/cmd/notifications/list.go index a12714a..521a9b1 100644 --- a/cmd/notifications/list.go +++ b/cmd/notifications/list.go @@ -69,7 +69,7 @@ func listNotifications(_ stdctx.Context, cmd *cli.Command, status []gitea.Notify all := ctx.Bool("mine") // This enforces pagination (see https://github.com/go-gitea/gitea/issues/16733) - listOpts := ctx.GetListOptions() + listOpts := flags.GetListOptions() if listOpts.Page == 0 { listOpts.Page = 1 } diff --git a/cmd/organizations/list.go b/cmd/organizations/list.go index 9251c33..2c7a267 100644 --- a/cmd/organizations/list.go +++ b/cmd/organizations/list.go @@ -33,7 +33,7 @@ func RunOrganizationList(_ stdctx.Context, cmd *cli.Command) error { client := ctx.Login.Client() userOrganizations, _, err := client.ListUserOrgs(ctx.Login.User, gitea.ListOrgsOptions{ - ListOptions: ctx.GetListOptions(), + ListOptions: flags.GetListOptions(), }) if err != nil { return err diff --git a/cmd/releases/list.go b/cmd/releases/list.go index 82bf7d3..431e43a 100644 --- a/cmd/releases/list.go +++ b/cmd/releases/list.go @@ -35,7 +35,7 @@ func RunReleasesList(_ stdctx.Context, cmd *cli.Command) error { ctx.Ensure(context.CtxRequirement{RemoteRepo: true}) releases, _, err := ctx.Login.Client().ListReleases(ctx.Owner, ctx.Repo, gitea.ListReleasesOptions{ - ListOptions: ctx.GetListOptions(), + ListOptions: flags.GetListOptions(), }) if err != nil { return err diff --git a/cmd/repos/list.go b/cmd/repos/list.go index b0c84c4..bea0243 100644 --- a/cmd/repos/list.go +++ b/cmd/repos/list.go @@ -65,14 +65,14 @@ func RunReposList(_ stdctx.Context, cmd *cli.Command) error { return err } rps, _, err = client.SearchRepos(gitea.SearchRepoOptions{ - ListOptions: teaCmd.GetListOptions(), + ListOptions: flags.GetListOptions(), StarredByUserID: user.ID, }) } else if teaCmd.Bool("watched") { rps, _, err = client.GetMyWatchedRepos() // TODO: this does not expose pagination.. } else { rps, _, err = client.ListMyRepos(gitea.ListReposOptions{ - ListOptions: teaCmd.GetListOptions(), + ListOptions: flags.GetListOptions(), }) } diff --git a/cmd/repos/search.go b/cmd/repos/search.go index 2489b42..9b8ad3b 100644 --- a/cmd/repos/search.go +++ b/cmd/repos/search.go @@ -109,7 +109,7 @@ func runReposSearch(_ stdctx.Context, cmd *cli.Command) error { } rps, _, err := client.SearchRepos(gitea.SearchRepoOptions{ - ListOptions: teaCmd.GetListOptions(), + ListOptions: flags.GetListOptions(), OwnerID: ownerID, IsPrivate: isPrivate, IsArchived: isArchived, diff --git a/modules/context/context.go b/modules/context/context.go index 52621e6..aec5592 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -13,7 +13,6 @@ import ( "strings" "time" - "code.gitea.io/sdk/gitea" "code.gitea.io/tea/modules/config" "code.gitea.io/tea/modules/git" "code.gitea.io/tea/modules/utils" @@ -35,22 +34,6 @@ type TeaContext struct { LocalRepo *git.TeaRepo // is set if flags specified a local repo via --repo, or if $PWD is a git repo } -// GetListOptions return ListOptions based on PaginationFlags -func (ctx *TeaContext) GetListOptions() gitea.ListOptions { - page := ctx.Int("page") - limit := ctx.Int("limit") - if limit < 0 { - limit = 0 - } - if limit != 0 && page == 0 { - page = 1 - } - return gitea.ListOptions{ - Page: page, - PageSize: limit, - } -} - // GetRemoteRepoHTMLURL returns the web-ui url of the remote repo, // after ensuring a remote repo is present in the context. func (ctx *TeaContext) GetRemoteRepoHTMLURL() string { diff --git a/modules/interact/comments.go b/modules/interact/comments.go index 5b96056..42c7dcc 100644 --- a/modules/interact/comments.go +++ b/modules/interact/comments.go @@ -8,6 +8,7 @@ import ( "os" "code.gitea.io/sdk/gitea" + "code.gitea.io/tea/cmd/flags" "code.gitea.io/tea/modules/context" "code.gitea.io/tea/modules/print" "code.gitea.io/tea/modules/theme" @@ -20,7 +21,7 @@ import ( // If that flag is unset, and output is not piped, prompts the user first. func ShowCommentsMaybeInteractive(ctx *context.TeaContext, idx int64, totalComments int) error { if ctx.Bool("comments") { - opts := gitea.ListIssueCommentOptions{ListOptions: ctx.GetListOptions()} + opts := gitea.ListIssueCommentOptions{ListOptions: flags.GetListOptions()} c := ctx.Login.Client() comments, _, err := c.ListIssueComments(ctx.Owner, ctx.Repo, idx, opts) if err != nil { @@ -39,7 +40,7 @@ func ShowCommentsMaybeInteractive(ctx *context.TeaContext, idx int64, totalComme // ShowCommentsPaginated prompts if issue/pr comments should be shown and continues to do so. func ShowCommentsPaginated(ctx *context.TeaContext, idx int64, totalComments int) error { c := ctx.Login.Client() - opts := gitea.ListIssueCommentOptions{ListOptions: ctx.GetListOptions()} + opts := gitea.ListIssueCommentOptions{ListOptions: flags.GetListOptions()} prompt := "show comments?" commentsLoaded := 0 diff --git a/modules/interact/pull_merge.go b/modules/interact/pull_merge.go index b0e7351..a47264e 100644 --- a/modules/interact/pull_merge.go +++ b/modules/interact/pull_merge.go @@ -7,6 +7,7 @@ import ( "fmt" "strings" + "code.gitea.io/tea/cmd/flags" "code.gitea.io/tea/modules/context" "code.gitea.io/tea/modules/task" "code.gitea.io/tea/modules/utils" @@ -43,7 +44,7 @@ func getPullIndex(ctx *context.TeaContext, branch string) (int64, error) { c := ctx.Login.Client() opts := gitea.ListPullRequestsOptions{ State: gitea.StateOpen, - ListOptions: ctx.GetListOptions(), + ListOptions: flags.GetListOptions(), } selected := "" loadMoreOption := "PR not found? Load more PRs..."