Merge pull request #283: feat(pr): 在列表输出中显示 PR 编号

# Conflicts: # shortcuts/pr/pr.go # shortcuts/pr/pr_test.go
2026-07-14 21:57:59 +08:00 · 2026-07-14 21:57:59 +08:00 · a2f8e1a01a
parent 8a60844342 c9aea1d983
commit a2f8e1a01a
6 changed files with 127 additions and 186 deletions
--- a/README.md
+++ b/README.md
@ -412,6 +412,9 @@ gitlink-cli label +delete --owner Gitlink --repo forgeplus -i 42
 # List PRs
 gitlink-cli pr +list --owner Gitlink --repo forgeplus

+# List PRs with the user-facing PR number column
+gitlink-cli pr +list --owner Gitlink --repo forgeplus --format table
+
 # Create a PR (same-repo branch)
 gitlink-cli pr +create --owner Gitlink --repo forgeplus -t "feat: Search feature" --head feature/search --base master

--- a/doc/changes/pr-list-number-column.md
+++ b/doc/changes/pr-list-number-column.md
@ -0,0 +1,13 @@
+## PR list output now shows the user-facing PR number
+
+`pr +list` already returned the GitLink PR sequence as `index`, but the default
+table output did not make that value easy to spot. This change copies the same
+value into a stable `number` field during list normalization and prioritizes the
+`number` column in table rendering.
+
+As a result:
+
+- `gitlink-cli pr +list --format table` shows the PR number in a dedicated
+  leading column.
+- JSON and YAML output also include `number`, making the list output align with
+  `pr +view --id <number>` semantics and with the PR number shown in the web UI.
--- a/internal/output/formatter.go
+++ b/internal/output/formatter.go
@ -198,7 +198,7 @@ func printMapTable(w io.Writer, m map[string]interface{}) error {
 func collectKeys(m map[string]interface{}) []string {
 	keys := make([]string, 0, len(m))
 	// Prefer common keys first
-	priority := []string{"id", "name", "login", "title", "status", "state", "created_at", "updated_at"}
+	priority := []string{"number", "id", "name", "login", "title", "status", "state", "created_at", "updated_at"}
 	seen := map[string]bool{}
 	for _, k := range priority {
 		if _, ok := m[k]; ok {
--- a/internal/output/formatter_test.go
+++ b/internal/output/formatter_test.go
@ -271,6 +271,7 @@ func TestHasComplexValues(t *testing.T) {

 func TestCollectKeys(t *testing.T) {
 	m := map[string]interface{}{
+		"number":     float64(3),
 		"title":      "test",
 		"id":         float64(1),
 		"status":     "open",
@ -278,17 +279,20 @@ func TestCollectKeys(t *testing.T) {
 	}
 	keys := collectKeys(m)
 	// Priority keys should come first
-	if len(keys) != 4 {
-		t.Fatalf("expected 4 keys, got %d", len(keys))
+	if len(keys) != 5 {
+		t.Fatalf("expected 5 keys, got %d", len(keys))
 	}
-	if keys[0] != "id" {
-		t.Fatalf("first key should be 'id', got %q", keys[0])
+	if keys[0] != "number" {
+		t.Fatalf("first key should be 'number', got %q", keys[0])
 	}
-	if keys[1] != "title" {
-		t.Fatalf("second key should be 'title', got %q", keys[1])
+	if keys[1] != "id" {
+		t.Fatalf("second key should be 'id', got %q", keys[1])
 	}
-	if keys[2] != "status" {
-		t.Fatalf("third key should be 'status', got %q", keys[2])
+	if keys[2] != "title" {
+		t.Fatalf("third key should be 'title', got %q", keys[2])
+	}
+	if keys[3] != "status" {
+		t.Fatalf("fourth key should be 'status', got %q", keys[3])
 	}
 }

--- a/shortcuts/pr/pr.go
+++ b/shortcuts/pr/pr.go
@ -38,8 +38,6 @@ 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")},
@ -54,9 +52,6 @@ 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"))
@ -91,6 +86,7 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut {
 				if err != nil {
 					return err
 				}
+				normalizePullRequestListNumbers(env)
 				return ctx.Output(env)
 			},
 		},
@ -475,30 +471,31 @@ 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
+func normalizePullRequestListNumbers(env *output.Envelope) {
+	if env == nil {
+		return
 	}

-	pr, ok := env.Data.(map[string]interface{})
+	data, ok := env.Data.(map[string]interface{})
 	if !ok {
-		return fmt.Errorf("unexpected PR response format")
+		return
 	}

-	if normalizedNumber := firstPullRequestNumber(pr); normalizedNumber != nil {
-		pr["number"] = normalizedNumber
+	pulls, ok := data["pulls"].([]interface{})
+	if !ok {
+		return
 	}

-	env.Data, env.Meta = wrapPullRequestListByNumberResult(pr)
-	return ctx.Output(env)
+	for i, item := range pulls {
+		pr, ok := item.(map[string]interface{})
+		if !ok {
+			continue
+		}
+		if number := firstPullRequestNumber(pr); number != nil {
+			pr["number"] = number
+		}
+		pulls[i] = pr
+	}
 }

 func enrichPullRequestClosedAt(ctx *common.RuntimeContext, env *output.Envelope) error {
@ -599,16 +596,3 @@ func firstPullRequestNumber(pr map[string]interface{}) interface{} {
 	}
 	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,
-		}
-}
--- a/shortcuts/pr/pr_test.go
+++ b/shortcuts/pr/pr_test.go
@ -5,10 +5,6 @@ import (
 	"fmt"
 	"net/http"
 	"net/http/httptest"
-	"os"
-	"os/exec"
-	"path/filepath"
-	"strings"
 	"testing"

 	"github.com/gitlink-org/gitlink-cli/internal/client"
@ -16,17 +12,31 @@ import (
 	"github.com/gitlink-org/gitlink-cli/shortcuts/common"
 )

-func TestPRCommentPostsToPullJournals(t *testing.T) {
+func TestPRCommentPostsToCorrectIssueJournal(t *testing.T) {
 	var journalPayload map[string]interface{}
+	var journalPath string
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if r.Method != "POST" || r.URL.Path != "/v1/owner/repo/pulls/13/journals.json" {
+		switch {
+		case r.Method == "GET" && r.URL.Path == "/owner/repo/pulls/13.json":
+			writeJSON(t, w, map[string]interface{}{
+				"issue": map[string]interface{}{
+					"id":      float64(142301),
+					"subject": "test PR",
+				},
+				"pull_request": map[string]interface{}{
+					"id": float64(14791),
+				},
+			})
+		case r.Method == "POST" && r.URL.Path == "/v1/owner/repo/issues/142301/journals.json":
+			journalPath = r.URL.Path
+			journalPayload = decodeJSON(t, r)
+			writeJSON(t, w, map[string]interface{}{
+				"id":      float64(12345),
+				"message": "评论成功",
+			})
+		default:
 			t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path)
 		}
-		journalPayload = decodeJSON(t, r)
-		writeJSON(t, w, map[string]interface{}{
-			"id":   float64(12345),
-			"note": "LGTM, looks good!",
-		})
 	}))
 	defer server.Close()

@ -37,7 +47,11 @@ func TestPRCommentPostsToPullJournals(t *testing.T) {
 	if err != nil {
 		t.Fatalf("comment shortcut failed: %v", err)
 	}
-	assertEqual(t, journalPayload["note"], "LGTM, looks good!")
+
+	if journalPath == "" {
+		t.Fatal("journal endpoint was not called")
+	}
+	assertEqual(t, journalPayload["notes"], "LGTM, looks good!")
 }

 func TestPRCommentFailsWhenPRNotFound(t *testing.T) {
@ -59,6 +73,25 @@ func TestPRCommentFailsWhenPRNotFound(t *testing.T) {
 	}
 }

+func TestPRCommentFailsWhenIssueFieldMissing(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		writeJSON(t, w, map[string]interface{}{
+			"pull_request": map[string]interface{}{
+				"id": float64(14791),
+			},
+		})
+	}))
+	defer server.Close()
+
+	err := runPRShortcut(t, server, "comment", map[string]string{
+		"id":   "13",
+		"body": "test",
+	})
+	if err == nil {
+		t.Fatal("expected error when issue field is missing, got nil")
+	}
+}
+
 // --- list ---

 func TestPRList(t *testing.T) {
@ -142,87 +175,48 @@ 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",
+func TestNormalizePullRequestListNumbersCopiesIndex(t *testing.T) {
+	env := &output.Envelope{
+		OK: true,
+		Data: map[string]interface{}{
+			"pulls": []interface{}{
+				map[string]interface{}{
+					"id":    float64(11),
+					"index": float64(7),
+					"title": "feat: show number",
+				},
+			},
 		},
 	}
-	if got := pullRequestListNumberArg(ctx); got != "15" {
-		t.Fatalf("pullRequestListNumberArg() = %q, want 15", got)
-	}
+
+	normalizePullRequestListNumbers(env)
+
+	data := env.Data.(map[string]interface{})
+	pulls := data["pulls"].([]interface{})
+	pr := pulls[0].(map[string]interface{})
+	assertEqual(t, pr["number"], float64(7))
 }

-func TestWrapPullRequestListByNumberResult(t *testing.T) {
-	pr := map[string]interface{}{
-		"id":     float64(303),
-		"number": float64(88),
-		"title":  "feat: wrapped number",
+func TestNormalizePullRequestListNumbersKeepsExistingNumber(t *testing.T) {
+	env := &output.Envelope{
+		OK: true,
+		Data: map[string]interface{}{
+			"pulls": []interface{}{
+				map[string]interface{}{
+					"number": float64(9),
+					"index":  float64(7),
+					"title":  "feat: keep number",
+				},
+			},
+		},
 	}

-	data, meta := wrapPullRequestListByNumberResult(pr)
+	normalizePullRequestListNumbers(env)

-	assertEqual(t, data["total_count"], 1)
-	assertEqual(t, data["page"], 1)
-	assertEqual(t, data["limit"], 1)
+	data := env.Data.(map[string]interface{})
 	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)
+	pr := pulls[0].(map[string]interface{})
+	assertEqual(t, pr["number"], float64(9))
 }

 // --- create ---
@ -581,60 +575,3 @@ func assertEqual(t *testing.T, got interface{}, want interface{}) {
 		t.Fatalf("got %v (%T), want %v (%T)", got, got, want, want)
 	}
 }
-
-func TestPRCheckoutFetchesHeadBranch(t *testing.T) {
-	if _, err := exec.LookPath("git"); err != nil {
-		t.Skip("git not available")
-	}
-	// Upstream repo with a PR head branch.
-	upstream := t.TempDir()
-	run := func(args ...string) {
-		t.Helper()
-		if out, err := exec.Command("git", args...).CombinedOutput(); err != nil {
-			t.Fatalf("git %v: %v (%s)", args, err, out)
-		}
-	}
-	run("init", "-q", "--initial-branch=master", upstream)
-	run("-C", upstream, "-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", "--allow-empty", "-m", "init")
-	run("-C", upstream, "branch", "feat/x")
-
-	// Local clone where checkout happens.
-	local := filepath.Join(t.TempDir(), "local")
-	run("clone", "-q", upstream, local)
-
-	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if r.URL.Path != "/v1/owner/repo/pulls/42.json" {
-			t.Fatalf("unexpected path: %s", r.URL.Path)
-		}
-		writeJSON(t, w, map[string]interface{}{"head": "feat/x", "base": "master"})
-	}))
-	defer server.Close()
-
-	cwd, _ := os.Getwd()
-	if err := os.Chdir(local); err != nil {
-		t.Fatal(err)
-	}
-	defer os.Chdir(cwd)
-
-	if err := runPRShortcut(t, server, "checkout", map[string]string{"id": "42"}); err != nil {
-		t.Fatalf("checkout shortcut failed: %v", err)
-	}
-	out, err := exec.Command("git", "-C", local, "branch", "--show-current").Output()
-	if err != nil {
-		t.Fatal(err)
-	}
-	if got := strings.TrimSpace(string(out)); got != "feat/x" {
-		t.Fatalf("current branch = %q, want feat/x", got)
-	}
-}
-
-func TestPRCheckoutMissingHead(t *testing.T) {
-	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		writeJSON(t, w, map[string]interface{}{"base": "master"})
-	}))
-	defer server.Close()
-
-	if err := runPRShortcut(t, server, "checkout", map[string]string{"id": "42"}); err == nil {
-		t.Fatal("expected error when head missing")
-	}
-}