From b2bab268d76ae57472c7e7af42a20cbcec53d24f Mon Sep 17 00:00:00 2001 From: ongolk <238961+ongolk@noreply.gitea.com> Date: Tue, 8 Sep 2026 19:20:40 +0000 Subject: [PATCH] Make `tea pulls create` honor `--output json` (#1111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem `tea pulls create` accepts `--output` (the flag parses successfully) but never reads it — the action always prints glamour-rendered markdown regardless of the requested format. That is a trap for anyone scripting the CLI: `tea pr create --output json | jq .url` quietly feeds jq a markdown document. The current output is also hostile to URL-scraping consumers even without `--output`: glamour autolinks the bare PR URL into an OSC 8 terminal hyperlink, so piped stdout contains ``` \x1b]8;;https://host/owner/repo/pulls/33\x1b\\https://host/owner/repo/pulls/33\x1b]8;;\x1b\\ ``` instead of a plain URL (repro: `tea pr create ... | cat -v`). ## Why the flag parses but does nothing `create` itself does not declare `--output`: its flag set (`IssuePRCreateFlags`) carries no `OutputFlag`. The flag parses anyway because urfave/cli v3 resolves flags through `Command.lookupAppliedFlag`, which searches `appliedFlags` — "local flags for current command **or persistent flags from ancestors**". The parent `pulls` command carries `--output` via `AllDefaultFlags`, so the flag reaches the subcommand's parser while being absent from `create --help` — and was never consulted by the action. ## What this changes - `task.CreatePull` now returns the created `*gitea.PullRequest` instead of printing it. - `runPullsCreate` switches on `--output`, mirroring the existing detail-command precedent (`RunPullsDetails` in `cmd/pulls.go`): `--output json` emits a lean JSON object; any other value (or no flag at all) falls through to the previous `print.PullDetails` rendering, byte-identical to before. - Lean JSON shape, since a freshly created PR has no reviews/comments/CI yet: `index`, `title`, `url`, `state`, `base`, `head`. - `--agit` combined with `--output` now fails fast with an explicit error before any API call or `git push`: the agit flow creates the PR server-side via push and returns no object to print. - The interactive path is untouched — it only triggers when zero flags are set, so `--output` can never be active there. Example: ``` $ tea pr create --output json --title "fix: thing" | jq -r .url https://gitea.example.com/owner/repo/pulls/33 ``` --------- Co-authored-by: Danilo Sousa Reviewed-on: https://gitea.com/gitea/tea/pulls/1111 Reviewed-by: Lunny Xiao Co-authored-by: ongolk <238961+ongolk@noreply.gitea.com> --- cmd/pulls/create.go | 49 ++++++++++++++++++++++++++++++++- cmd/pulls/create_app_test.go | 48 ++++++++++++++++++++++++++++++++ cmd/pulls/create_test.go | 45 ++++++++++++++++++++++++++++++ modules/interact/pull_create.go | 10 ++++++- modules/task/pull_create.go | 25 ++++++++--------- 5 files changed, 162 insertions(+), 15 deletions(-) create mode 100644 cmd/pulls/create_app_test.go create mode 100644 cmd/pulls/create_test.go diff --git a/cmd/pulls/create.go b/cmd/pulls/create.go index fc3e7c12..6740c974 100644 --- a/cmd/pulls/create.go +++ b/cmd/pulls/create.go @@ -5,6 +5,9 @@ package pulls import ( stdctx "context" + "encoding/json" + "fmt" + "io" gitea "gitea.dev/sdk" "github.com/urfave/cli/v3" @@ -12,6 +15,7 @@ import ( "gitea.dev/tea/cmd/flags" "gitea.dev/tea/modules/context" "gitea.dev/tea/modules/interact" + "gitea.dev/tea/modules/print" "gitea.dev/tea/modules/task" "gitea.dev/tea/modules/utils" ) @@ -80,6 +84,12 @@ func runPullsCreate(requestCtx stdctx.Context, cmd *cli.Command) error { return nil } + // agit flow creates the PR via git push and returns no PR object, so + // --output cannot be honored there; fail fast before any API calls + if ctx.Bool("agit") && ctx.IsSet("output") { + return fmt.Errorf("--output cannot be combined with --agit: the PR is created via git push, so no pull request object is available to print") + } + // else use args to create PR opts, err := flags.GetIssuePRCreateFlags(requestCtx, ctx) if err != nil { @@ -108,7 +118,7 @@ func runPullsCreate(requestCtx stdctx.Context, cmd *cli.Command) error { allowMaintainerEdits = gitea.OptionalBool(ctx.Bool("allow-maintainer-edits")) } - return task.CreatePull( + pr, err := task.CreatePull( requestCtx, ctx, ctx.String("base"), @@ -116,4 +126,41 @@ func runPullsCreate(requestCtx stdctx.Context, cmd *cli.Command) error { allowMaintainerEdits, opts, ) + if err != nil { + return err + } + + if ctx.IsSet("output") { + switch ctx.String("output") { + case "json": + return writeCreatedPullAsJSON(ctx.Writer, pr) + } + } + + print.PullDetails(pr, nil, nil) + + return nil +} + +// createdPullJSON is the machine-readable representation of a freshly +// created pull request. A new PR has no reviews, comments or CI yet, so +// this is intentionally leaner than the detail view's pullData (cmd/pulls.go). +type createdPullJSON struct { + Index int64 `json:"index"` + Title string `json:"title"` + URL string `json:"url"` + State gitea.StateType `json:"state"` + Base string `json:"base"` + Head string `json:"head"` +} + +func writeCreatedPullAsJSON(w io.Writer, pr *gitea.PullRequest) error { + return json.NewEncoder(w).Encode(createdPullJSON{ + Index: pr.Index, + Title: pr.Title, + URL: pr.HTMLURL, + State: pr.State, + Base: pr.Base.Ref, + Head: pr.Head.Ref, + }) } diff --git a/cmd/pulls/create_app_test.go b/cmd/pulls/create_app_test.go new file mode 100644 index 00000000..c22c1f51 --- /dev/null +++ b/cmd/pulls/create_app_test.go @@ -0,0 +1,48 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pulls_test + +import ( + "context" + "testing" + + "gitea.dev/tea/cmd" + "gitea.dev/tea/modules/config" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestPullsCreateAgitOutputRejected verifies that --output (parsed via the +// urfave/cli v3 ancestor-flag cascade, since create itself does not declare +// it) is rejected for the agit flow before any API call or git push happens. +func TestPullsCreateAgitOutputRejected(t *testing.T) { + config.SetConfigForTesting(config.LocalConfig{ + Logins: []config.Login{{ + Name: "testLogin", + URL: "https://gitea.example.com", + Token: "test-token", + User: "testUser", + Default: true, + }}, + }) + t.Cleanup(func() { + config.SetConfigForTesting(config.LocalConfig{}) + }) + + app := cmd.App() + args := []string{ + "tea", "pulls", "create", + "--agit", + "--output", "json", + "--head", "topic-branch", + "--title", "test", + "--login", "testLogin", + "--repo", "user/repo", + } + + err := app.Run(context.Background(), args) + require.Error(t, err) + assert.Contains(t, err.Error(), "--output cannot be combined with --agit") +} diff --git a/cmd/pulls/create_test.go b/cmd/pulls/create_test.go new file mode 100644 index 00000000..93f9774d --- /dev/null +++ b/cmd/pulls/create_test.go @@ -0,0 +1,45 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pulls + +import ( + "bytes" + "encoding/json" + "testing" + + gitea "gitea.dev/sdk" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestWriteCreatedPullAsJSON(t *testing.T) { + pr := &gitea.PullRequest{ + Index: 33, + Title: "test title", + HTMLURL: "https://gitea.example.com/owner/repo/pulls/33", + State: gitea.StateOpen, + Base: &gitea.PRBranchInfo{Ref: "main"}, + Head: &gitea.PRBranchInfo{Ref: "feature"}, + } + + var buf bytes.Buffer + require.NoError(t, writeCreatedPullAsJSON(&buf, pr)) + + var got map[string]any + require.NoError(t, json.Unmarshal(buf.Bytes(), &got)) + + assert.Equal(t, float64(33), got["index"]) + assert.Equal(t, "test title", got["title"]) + assert.Equal(t, "https://gitea.example.com/owner/repo/pulls/33", got["url"]) + assert.Equal(t, "open", got["state"]) + assert.Equal(t, "main", got["base"]) + assert.Equal(t, "feature", got["head"]) + + // exactly the lean field set, nothing extra + assert.Len(t, got, 6) + + // machine-readable output must not contain terminal escape sequences + assert.NotContains(t, buf.String(), "\x1b") +} diff --git a/modules/interact/pull_create.go b/modules/interact/pull_create.go index 0d8a1f3b..4cdb480f 100644 --- a/modules/interact/pull_create.go +++ b/modules/interact/pull_create.go @@ -8,6 +8,7 @@ import ( gitea "gitea.dev/sdk" "gitea.dev/tea/modules/context" + "gitea.dev/tea/modules/print" "gitea.dev/tea/modules/task" "gitea.dev/tea/modules/theme" @@ -134,11 +135,18 @@ func CreatePull(requestCtx stdctx.Context, ctx *context.TeaContext) (err error) return err } - return task.CreatePull( + pr, err := task.CreatePull( requestCtx, ctx, base, head, &allowMaintainerEdits, &opts) + if err != nil { + return err + } + + print.PullDetails(pr, nil, nil) + + return nil } diff --git a/modules/task/pull_create.go b/modules/task/pull_create.go index f35f5783..2f554730 100644 --- a/modules/task/pull_create.go +++ b/modules/task/pull_create.go @@ -14,7 +14,6 @@ import ( "gitea.dev/tea/modules/config" "gitea.dev/tea/modules/context" local_git "gitea.dev/tea/modules/git" - "gitea.dev/tea/modules/print" "gitea.dev/tea/modules/utils" ) @@ -24,24 +23,26 @@ var ( consecutive = regexp.MustCompile(`[\s]{2,}`) ) -// CreatePull creates a PR in the given repo and prints the result -func CreatePull(requestCtx stdctx.Context, ctx *context.TeaContext, base, head string, allowMaintainerEdits *bool, opts *gitea.CreateIssueOption) (err error) { +// CreatePull creates a PR in the given repo and returns the created PR +func CreatePull(requestCtx stdctx.Context, ctx *context.TeaContext, base, head string, allowMaintainerEdits *bool, opts *gitea.CreateIssueOption) (*gitea.PullRequest, error) { + var err error + // default is default branch if len(base) == 0 { base, err = GetDefaultPRBase(requestCtx, ctx.Login, ctx.Owner, ctx.Repo) if err != nil { - return err + return nil, err } } // default is current one if len(head) == 0 { if ctx.LocalRepo == nil { - return fmt.Errorf("no local git repo detected, please specify head branch") + return nil, fmt.Errorf("no local git repo detected, please specify head branch") } headOwner, headBranch, err := GetDefaultPRHead(ctx.LocalRepo) if err != nil { - return err + return nil, err } head = GetHeadSpec(headOwner, headBranch, ctx.Owner) @@ -49,7 +50,7 @@ func CreatePull(requestCtx stdctx.Context, ctx *context.TeaContext, base, head s // head & base may not be the same if head == base { - return fmt.Errorf("can't create PR from %s to %s", head, base) + return nil, fmt.Errorf("can't create PR from %s to %s", head, base) } // default is head branch name @@ -58,7 +59,7 @@ func CreatePull(requestCtx stdctx.Context, ctx *context.TeaContext, base, head s } // title is required if len(opts.Title) == 0 { - return fmt.Errorf("title is required") + return nil, fmt.Errorf("title is required") } client := ctx.Login.Client() @@ -74,7 +75,7 @@ func CreatePull(requestCtx stdctx.Context, ctx *context.TeaContext, base, head s Deadline: opts.Deadline, }) if err != nil { - return fmt.Errorf("could not create PR from %s to %s:%s: %s", head, ctx.Owner, base, err) + return nil, fmt.Errorf("could not create PR from %s to %s:%s: %s", head, ctx.Owner, base, err) } if allowMaintainerEdits != nil && pr.AllowMaintainerEdit != *allowMaintainerEdits { @@ -82,13 +83,11 @@ func CreatePull(requestCtx stdctx.Context, ctx *context.TeaContext, base, head s AllowMaintainerEdit: allowMaintainerEdits, }) if err != nil { - return fmt.Errorf("could not enable maintainer edit on pull: %v", err) + return nil, fmt.Errorf("could not enable maintainer edit on pull: %v", err) } } - print.PullDetails(pr, nil, nil) - - return err + return pr, nil } // GetDefaultPRBase retrieves the default base branch for the given repo