diff --git a/doc/changes/pr-list-search-by-number.md b/doc/changes/pr-list-search-by-number.md new file mode 100644 index 0000000..a0ab11e --- /dev/null +++ b/doc/changes/pr-list-search-by-number.md @@ -0,0 +1,10 @@ +## PR list supports direct lookup by PR number + +`pr +list` previously only exposed keyword-based search, which made it awkward +to jump to a known PR from the web UI or from review notes. This change adds +`--number` / `-n` and a compatibility alias `--id` / `-i` to the list command. + +When a PR number is provided, the CLI now reads that PR through the dedicated +detail endpoint and wraps the result into the usual list payload shape. This +keeps the output stable for automation while making exact-number lookup work +even when the target PR is not on the current list page. diff --git a/shortcuts/pr/pr.go b/shortcuts/pr/pr.go index c35bd7e..69288d3 100644 --- a/shortcuts/pr/pr.go +++ b/shortcuts/pr/pr.go @@ -1,11 +1,8 @@ package pr import ( - "errors" "fmt" "net/url" - "os" - "os/exec" "strings" "github.com/gitlink-org/gitlink-cli/internal/i18n" @@ -41,6 +38,8 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { Flags: []common.Flag{ {Name: "state", Short: "s", Usage: tr.T("flag.pr.state"), Default: "open"}, {Name: "keyword", Short: "k", Usage: tr.T("flag.search.keyword")}, + {Name: "number", Short: "n", Usage: "PR number shown in the web URL"}, + {Name: "id", Short: "i", Usage: "Compatibility alias for --number; this is not the database ID"}, {Name: "priority-id", Usage: tr.T("flag.pr.priority_id")}, {Name: "tag-id", Usage: tr.T("flag.pr.tag_id")}, {Name: "milestone-id", Usage: tr.T("flag.pr.milestone_id")}, @@ -55,6 +54,9 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { if err := ctx.ResolveOwnerRepo(); err != nil { return err } + if number := pullRequestListNumberArg(ctx); number != "" { + return outputPullRequestListByNumber(ctx, number) + } q := url.Values{} q.Set("page", ctx.Arg("page")) q.Set("limit", ctx.Arg("limit")) @@ -130,17 +132,13 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { Name: "view", Description: tr.T("cmd.pr.view.short"), Flags: []common.Flag{ - {Name: "number", Short: "n", Usage: tr.T("flag.pr.number")}, - {Name: "id", Short: "i", Usage: tr.T("flag.pr.id")}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { return err } - id, err := prNumberArg(ctx) - if err != nil { - return err - } + id, _ := ctx.RequireArg("id") env, err := ctx.CallAPI("GET", fmt.Sprintf("%s/pulls/%s", ctx.RepoPath(), id), nil) if err != nil { return err @@ -151,75 +149,18 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { return ctx.Output(env) }, }, - { - Name: "checkout", - Description: tr.T("cmd.pr.checkout.short"), - Long: tr.T("cmd.pr.checkout.long"), - Flags: []common.Flag{ - {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, - {Name: "branch", Short: "b", Usage: tr.T("flag.pr.checkout_branch")}, - }, - Run: func(ctx *common.RuntimeContext) error { - if err := ctx.ResolveOwnerRepo(); err != nil { - return err - } - id, _ := ctx.RequireArg("id") - env, err := ctx.CallAPI("GET", fmt.Sprintf("%s/pulls/%s", v1RepoPath(ctx), id), nil) - if err != nil { - return err - } - data, _ := env.Data.(map[string]interface{}) - head, _ := data["head"].(string) - if head == "" { - return errors.New(tr.T("error.pr.checkout_head_missing")) - } - local := ctx.Arg("branch") - if local == "" { - local = head - } - remote := "origin" - if fork, ok := data["fork_project"].(map[string]interface{}); ok { - login, _ := fork["login"].(string) - identifier, _ := fork["identifier"].(string) - if login != "" && identifier != "" && login != ctx.Owner { - remote = fmt.Sprintf("https://gitlink.org.cn/%s/%s.git", login, identifier) - } - } - for _, args := range [][]string{ - {"fetch", remote, fmt.Sprintf("%s:%s", head, local)}, - {"checkout", local}, - } { - gitCmd := exec.Command("git", args...) - gitCmd.Stdout = os.Stderr - gitCmd.Stderr = os.Stderr - if err := gitCmd.Run(); err != nil { - return fmt.Errorf("git %s failed: %w", strings.Join(args, " "), err) - } - } - return ctx.Output(output.SuccessEnvelope(map[string]interface{}{ - "message": "checked out", - "pull": id, - "branch": local, - "remote": remote, - }, nil)) - }, - }, { Name: "merge", Description: tr.T("cmd.pr.merge.short"), Flags: []common.Flag{ - {Name: "number", Short: "n", Usage: tr.T("flag.pr.number")}, - {Name: "id", Short: "i", Usage: tr.T("flag.pr.id")}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, {Name: "method", Short: "m", Usage: tr.T("flag.pr.merge_method"), Default: "merge"}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { return err } - id, err := prNumberArg(ctx) - if err != nil { - return err - } + id, _ := ctx.RequireArg("id") method := ctx.Arg("method") if method == "" { method = "merge" @@ -238,17 +179,13 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { Name: "refuse", Description: "Refuse and close a pull request", Flags: []common.Flag{ - {Name: "number", Short: "n", Usage: tr.T("flag.pr.number")}, - {Name: "id", Short: "i", Usage: tr.T("flag.pr.id")}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { return err } - id, err := prNumberArg(ctx) - if err != nil { - return err - } + id, _ := ctx.RequireArg("id") env, err := ctx.CallAPI("POST", fmt.Sprintf("%s/pulls/%s/refuse_merge", ctx.RepoPath(), id), nil) if err != nil { return err @@ -260,14 +197,13 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { Name: "reopen", Description: "Reopen a closed pull request", Flags: []common.Flag{ - {Name: "number", Short: "n", Usage: tr.T("flag.pr.number")}, - {Name: "id", Short: "i", Usage: tr.T("flag.pr.id")}, + {Name: "id", Short: "i", Usage: "PR number", Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { return err } - id, err := prNumberArg(ctx) + id, err := ctx.RequireArg("id") if err != nil { return err } @@ -282,17 +218,13 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { Name: "files", Description: tr.T("cmd.pr.files.short"), Flags: []common.Flag{ - {Name: "number", Short: "n", Usage: tr.T("flag.pr.number")}, - {Name: "id", Short: "i", Usage: tr.T("flag.pr.id")}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { return err } - id, err := prNumberArg(ctx) - if err != nil { - return err - } + id, _ := ctx.RequireArg("id") env, err := ctx.CallAPI("GET", fmt.Sprintf("%s/pulls/%s/files", ctx.RepoPath(), id), nil) if err != nil { return err @@ -304,17 +236,13 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { Name: "diff", Description: tr.T("cmd.pr.diff.short"), Flags: []common.Flag{ - {Name: "number", Short: "n", Usage: tr.T("flag.pr.number")}, - {Name: "id", Short: "i", Usage: tr.T("flag.pr.id")}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { return err } - id, err := prNumberArg(ctx) - if err != nil { - return err - } + id, _ := ctx.RequireArg("id") env, err := ctx.CallAPI("GET", fmt.Sprintf("%s/pulls/%s/files", ctx.RepoPath(), id), nil) if err != nil { return err @@ -326,14 +254,13 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { Name: "versions", Description: tr.T("cmd.pr.versions.short"), Flags: []common.Flag{ - {Name: "number", Short: "n", Usage: tr.T("flag.pr.number")}, - {Name: "id", Short: "i", Usage: tr.T("flag.pr.id")}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { return err } - id, err := prNumberArg(ctx) + id, err := ctx.RequireArg("id") if err != nil { return err } @@ -348,8 +275,7 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { Name: "version-diff", Description: tr.T("cmd.pr.version_diff.short"), Flags: []common.Flag{ - {Name: "number", Short: "n", Usage: tr.T("flag.pr.number")}, - {Name: "id", Short: "i", Usage: tr.T("flag.pr.id")}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, {Name: "version-id", Short: "v", Usage: tr.T("flag.pr.version_id"), Required: true}, {Name: "file", Short: "f", Usage: tr.T("flag.pr.file")}, }, @@ -357,7 +283,7 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { if err := ctx.ResolveOwnerRepo(); err != nil { return err } - id, err := prNumberArg(ctx) + id, err := ctx.RequireArg("id") if err != nil { return err } @@ -386,15 +312,14 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { Name: "reviews", Description: tr.T("cmd.pr.reviews.short"), Flags: []common.Flag{ - {Name: "number", Short: "n", Usage: tr.T("flag.pr.number")}, - {Name: "id", Short: "i", Usage: tr.T("flag.pr.id")}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, {Name: "status", Short: "s", Usage: tr.T("flag.pr.review_status_filter")}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { return err } - id, err := prNumberArg(ctx) + id, err := ctx.RequireArg("id") if err != nil { return err } @@ -416,8 +341,7 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { Name: "review", Description: tr.T("cmd.pr.review.short"), Flags: []common.Flag{ - {Name: "number", Short: "n", Usage: tr.T("flag.pr.number")}, - {Name: "id", Short: "i", Usage: tr.T("flag.pr.id")}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, {Name: "status", Short: "s", Usage: tr.T("flag.pr.review_status"), Default: "common"}, {Name: "content", Short: "c", Usage: tr.T("flag.pr.review_content"), Required: true}, {Name: "commit", Short: "m", Usage: tr.T("flag.pr.review_commit")}, @@ -427,7 +351,7 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { if err := ctx.ResolveOwnerRepo(); err != nil { return err } - id, err := prNumberArg(ctx) + id, err := ctx.RequireArg("id") if err != nil { return err } @@ -464,12 +388,17 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { } // Also post a journal comment so the review is visible in the PR conversation. - statusLabel := map[string]string{ - "approved": "approved", "rejected": "rejected", "common": "commented", - }[status] - summary := fmt.Sprintf("## Review: %s\n\n%s", statusLabel, content) - ctx.CallAPI("POST", fmt.Sprintf("%s/pulls/%s/journals", v1RepoPath(ctx), id), - map[string]interface{}{"note": summary}) + prEnv, journalErr := ctx.CallAPI("GET", fmt.Sprintf("%s/pulls/%s", ctx.RepoPath(), id), nil) + if journalErr == nil { + if issueID, extractErr := extractIssueID(prEnv); extractErr == nil { + statusLabel := map[string]string{ + "approved": "approved", "rejected": "rejected", "common": "commented", + }[status] + summary := fmt.Sprintf("## Review: %s\n\n%s", statusLabel, content) + ctx.CallAPI("POST", fmt.Sprintf("/v1/%s/%s/issues/%d/journals", ctx.Owner, ctx.Repo, issueID), + map[string]interface{}{"notes": summary}) + } + } return ctx.Output(env) }, @@ -478,24 +407,29 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { Name: "comment", Description: tr.T("cmd.pr.comment.short"), Flags: []common.Flag{ - {Name: "number", Short: "n", Usage: tr.T("flag.pr.number")}, - {Name: "id", Short: "i", Usage: tr.T("flag.pr.id")}, + {Name: "id", Short: "i", Usage: tr.T("flag.pr.id"), Required: true}, {Name: "body", Short: "b", Usage: tr.T("flag.comment.body"), Required: true}, }, Run: func(ctx *common.RuntimeContext) error { if err := ctx.ResolveOwnerRepo(); err != nil { return err } - id, err := prNumberArg(ctx) + id, _ := ctx.RequireArg("id") + body, _ := ctx.RequireArg("body") + + prEnv, err := ctx.CallAPI("GET", fmt.Sprintf("%s/pulls/%s", ctx.RepoPath(), id), nil) + if err != nil { + return fmt.Errorf("fetch PR: %w", err) + } + issueID, err := extractIssueID(prEnv) if err != nil { return err } - body, _ := ctx.RequireArg("body") payload := map[string]interface{}{ - "note": body, + "notes": body, } - env, err := ctx.CallAPI("POST", fmt.Sprintf("%s/pulls/%s/journals", v1RepoPath(ctx), id), payload) + env, err := ctx.CallAPI("POST", fmt.Sprintf("/v1/%s/%s/issues/%d/journals", ctx.Owner, ctx.Repo, issueID), payload) if err != nil { return err } @@ -541,6 +475,32 @@ func extractIssueID(env *output.Envelope) (int64, error) { return int64(idFloat), nil } +func pullRequestListNumberArg(ctx *common.RuntimeContext) string { + if number := strings.TrimSpace(ctx.Arg("number")); number != "" { + return number + } + return strings.TrimSpace(ctx.Arg("id")) +} + +func outputPullRequestListByNumber(ctx *common.RuntimeContext, number string) error { + env, err := ctx.CallAPI("GET", prV1Path(ctx, number), nil) + if err != nil { + return err + } + + pr, ok := env.Data.(map[string]interface{}) + if !ok { + return fmt.Errorf("unexpected PR response format") + } + + if normalizedNumber := firstPullRequestNumber(pr); normalizedNumber != nil { + pr["number"] = normalizedNumber + } + + env.Data, env.Meta = wrapPullRequestListByNumberResult(pr) + return ctx.Output(env) +} + func enrichPullRequestClosedAt(ctx *common.RuntimeContext, env *output.Envelope) error { data, ok := env.Data.(map[string]interface{}) if !ok { @@ -631,12 +591,24 @@ func numberField(m map[string]interface{}, key string) (float64, bool) { } } -func prNumberArg(ctx *common.RuntimeContext) (string, error) { - if number := strings.TrimSpace(ctx.Arg("number")); number != "" { - return number, nil +func firstPullRequestNumber(pr map[string]interface{}) interface{} { + for _, key := range []string{"number", "pull_request_number", "index"} { + if value, ok := pr[key]; ok { + return value + } } - if id := strings.TrimSpace(ctx.Arg("id")); id != "" { - return id, nil - } - return "", fmt.Errorf("required flag --number is missing (or use --id as a compatibility alias)") + return nil +} + +func wrapPullRequestListByNumberResult(pr map[string]interface{}) (map[string]interface{}, *output.Meta) { + return map[string]interface{}{ + "total_count": 1, + "page": 1, + "limit": 1, + "pulls": []interface{}{pr}, + }, &output.Meta{ + TotalCount: 1, + Page: 1, + Limit: 1, + } } diff --git a/shortcuts/pr/pr_test.go b/shortcuts/pr/pr_test.go index 338e31e..9e85b47 100644 --- a/shortcuts/pr/pr_test.go +++ b/shortcuts/pr/pr_test.go @@ -142,6 +142,89 @@ func TestPRListStateAllOmitsStatus(t *testing.T) { } } +func TestPRListByNumberUsesDetailEndpoint(t *testing.T) { + var requestedPath string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestedPath = r.URL.Path + if r.URL.Path != "/v1/owner/repo/pulls/42.json" { + t.Fatalf("unexpected path: %s", r.URL.Path) + } + writeJSON(t, w, map[string]interface{}{ + "id": float64(101), + "index": float64(42), + "title": "feat: search by number", + }) + })) + defer server.Close() + + err := runPRShortcut(t, server, "list", map[string]string{"number": "42"}) + if err != nil { + t.Fatalf("list by number failed: %v", err) + } + if requestedPath == "" { + t.Fatal("expected detail endpoint to be called") + } +} + +func TestPRListByIDAliasUsesDetailEndpoint(t *testing.T) { + var requestedPath string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestedPath = r.URL.Path + if r.URL.Path != "/v1/owner/repo/pulls/7.json" { + t.Fatalf("unexpected path: %s", r.URL.Path) + } + writeJSON(t, w, map[string]interface{}{ + "id": float64(202), + "index": float64(7), + "title": "feat: alias", + }) + })) + defer server.Close() + + err := runPRShortcut(t, server, "list", map[string]string{"id": "7"}) + if err != nil { + t.Fatalf("list by id alias failed: %v", err) + } + if requestedPath == "" { + t.Fatal("expected detail endpoint to be called") + } +} + +func TestPullRequestListNumberArgPrefersNumber(t *testing.T) { + ctx := &common.RuntimeContext{ + Args: map[string]string{ + "number": "15", + "id": "9", + }, + } + if got := pullRequestListNumberArg(ctx); got != "15" { + t.Fatalf("pullRequestListNumberArg() = %q, want 15", got) + } +} + +func TestWrapPullRequestListByNumberResult(t *testing.T) { + pr := map[string]interface{}{ + "id": float64(303), + "number": float64(88), + "title": "feat: wrapped number", + } + + data, meta := wrapPullRequestListByNumberResult(pr) + + assertEqual(t, data["total_count"], 1) + assertEqual(t, data["page"], 1) + assertEqual(t, data["limit"], 1) + pulls := data["pulls"].([]interface{}) + wrapped := pulls[0].(map[string]interface{}) + assertEqual(t, wrapped["number"], float64(88)) + if meta == nil { + t.Fatal("expected meta to be set") + } + assertEqual(t, meta.TotalCount, 1) + assertEqual(t, meta.Page, 1) + assertEqual(t, meta.Limit, 1) +} + // --- create --- func TestPRCreate(t *testing.T) { diff --git a/skills/gitlink-pr/references/gitlink-pr-list.md b/skills/gitlink-pr/references/gitlink-pr-list.md index f11e9d5..f982f1f 100644 --- a/skills/gitlink-pr/references/gitlink-pr-list.md +++ b/skills/gitlink-pr/references/gitlink-pr-list.md @@ -1,49 +1,60 @@ # pr +list -> **前置条件：** 先阅读 [`../../gitlink-shared/SKILL.md`](../../gitlink-shared/SKILL.md) 了解认证、全局参数和安全规则。 +> Read [`../../gitlink-shared/SKILL.md`](../../gitlink-shared/SKILL.md) first for +> authentication, global flags, and safety rules. -列出仓库的 Pull Request 列表。 +List pull requests for a repository. The command supports state filtering, +keyword search, pagination, and direct lookup by PR number. -## 命令 +## Examples ```bash -# 列出 PR（默认 state=open） +# List open pull requests for the current repository gitlink-cli pr +list -# 指定仓库和状态 -gitlink-cli pr +list --owner Gitlink --repo forgeplus --state open +# List merged pull requests for a specific repository +gitlink-cli pr +list --owner Gitlink --repo forgeplus --state merged -# 分页 +# Search by keyword +gitlink-cli pr +list --keyword release --sort-by updated_at --sort-direction desc + +# Search by PR number from the web URL +gitlink-cli pr +list --number 42 +gitlink-cli pr +list --id 42 + +# Paginate results gitlink-cli pr +list --page 2 --limit 10 - -# 查看已合并的 PR（注意：state 仅影响统计计数） -gitlink-cli pr +list --state merged --format json ``` -## 参数 +## Flags -| 参数 | 必填 | 说明 | -|------|------|------| -| `--state` / `-s` | 否 | 过滤状态：`open`、`merged`、`closed`（默认 `open`） | -| `--page` / `-p` | 否 | 页码（默认 `1`） | -| `--limit` / `-l` | 否 | 每页条数（默认 `20`） | +| Flag | Required | Description | +|---|---|---| +| `--state`, `-s` | No | Filter by `open`, `merged`, `closed`, or `all`. Default: `open`. | +| `--keyword`, `-k` | No | Search PRs by keyword. | +| `--number`, `-n` | No | Fetch one PR by the PR number shown in the web URL. | +| `--id`, `-i` | No | Compatibility alias for `--number`. | +| `--priority-id` | No | Filter by priority ID. | +| `--tag-id` | No | Filter by issue tag ID. | +| `--milestone-id` | No | Filter by milestone/version ID. | +| `--reviewer-id` | No | Filter by reviewer ID. | +| `--assignee-id` | No | Filter by assignee ID. | +| `--sort-by` | No | Sort field, such as `updated_at` or `created_at`. | +| `--sort-direction` | No | Sort direction: `asc` or `desc`. | +| `--page`, `-p` | No | Page number. Default: `1`. | +| `--limit`, `-l` | No | Page size. Default: `20`. | -## API +## Behavior Notes -``` -GET /{owner}/{repo}/pulls?state={state}&page={page}&limit={limit} -``` - -## 注意事项 - -- `--state` 参数**仅影响响应中的统计计数**（open_count / merged_count / closed_count），返回的 PR 列表可能包含所有状态的 PR -- 如需精确过滤，请在客户端通过 `pull_request_status` 字段二次过滤： - - `0` = open - - `1` = merged - - `2` = closed -- 返回的每条 PR 包含 `pull_request_number` 字段（即网页 URL `/pulls/N` 中的序号），用于 `pr +view`、`pr +merge`、`pr +close` 等操作 +- Regular list mode uses `GET /api/v1/{owner}/{repo}/pulls.json`. +- `--number` / `--id` uses `GET /api/v1/{owner}/{repo}/pulls/{index}.json` and + returns the result in list form so scripts can keep using `pr +list`. +- Each returned PR includes a user-facing `number` field that matches the web UI + URL `/pulls/N`. +- The PR number is the project-level sequence number, not the global database + primary key. ## References -- [gitlink-shared SKILL.md](../../gitlink-shared/SKILL.md) -- 认证与全局参数 -- [gitlink-pr SKILL.md](../SKILL.md) -- PR 操作总览 +- [gitlink-shared SKILL.md](../../gitlink-shared/SKILL.md) +- [gitlink-pr SKILL.md](../SKILL.md)