mirror of
https://gitea.com/gitea/tea.git
synced 2026-09-10 07:26:33 -04:00
Make tea pulls create honor --output json (#1111)
## 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 <code@danilosousa.net> Reviewed-on: https://gitea.com/gitea/tea/pulls/1111 Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: ongolk <238961+ongolk@noreply.gitea.com>
This commit is contained in:
parent
58931b5d17
commit
b2bab268d7
|
|
@ -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,
|
||||
})
|
||||
}
|
||||
|
|
|
|||
48
cmd/pulls/create_app_test.go
Normal file
48
cmd/pulls/create_app_test.go
Normal file
|
|
@ -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")
|
||||
}
|
||||
45
cmd/pulls/create_test.go
Normal file
45
cmd/pulls/create_test.go
Normal file
|
|
@ -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")
|
||||
}
|
||||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in a new issue