Remove pagination from context (#807)

Pagination related flags now write directly to ListOption struct and enforce non negative numbers.
Flag tests were added to cover the validation

Reviewed-on: https://gitea.com/gitea/tea/pulls/807
Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: TheFox0x7 <thefox0x7@gmail.com>
Co-committed-by: TheFox0x7 <thefox0x7@gmail.com>

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

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)
+		})
+	}
+}