mirror of
https://gitea.com/gitea/tea.git
synced 2026-10-26 19:11:25 +01:00
fix: report unknown --json fields with the fields a command outputs
tea issue list --json number,titel failed with "accepts 0 arg(s), received 1": a field list with an unknown name stayed a positional argument. A field-shaped word after --json now binds to it unless the command takes the word as an argument, and an unknown field lists the available fields. --json= is a usage error instead of printing text. --jq alone now implies --json, except on commands annotated tea.rawjson that filter raw JSON themselves; -q is short for --jq and -y for --yes. Fields are listed alphabetically, and terminal.JSONOutput can indent and color JSON for a terminal. Signed-off-by: bircni <bircni@icloud.com>
This commit is contained in:
4 files changed
+216
-42
No files matched your search
+50
-25
@@ -109,22 +109,16 @@ func NewRoot(deps Dependencies) *cobra.Command {
|
||||
flags.StringVarP(&a.options.repo, "repo", "R", "", "Repository owner/name or URL")
|
||||
flags.StringVar(&a.options.cwd, "cwd", "", "Working directory for repository discovery")
|
||||
flags.StringVar(&a.options.remote, "remote", "", "Git remote used for repository discovery")
|
||||
flags.StringVar(&a.options.json, "json", "", "Output JSON; optionally select comma-separated fields")
|
||||
flags.StringVar(&a.options.json, "json", "", "Output JSON with the specified comma-separated `fields`, or all fields")
|
||||
flags.Lookup("json").NoOptDefVal = "*"
|
||||
flags.StringVar(&a.options.jq, "jq", "", "Filter JSON with a jq expression")
|
||||
flags.StringVarP(&a.options.jq, "jq", "q", "", "Filter JSON output using a jq `expression`; implies --json")
|
||||
flags.BoolVar(&a.options.noInput, "no-input", false, "Disable prompts and implicit editor/browser interactions")
|
||||
flags.BoolVar(&a.options.interactive, "interactive", false, "Allow prompts with machine output")
|
||||
flags.BoolVar(&a.options.yes, "yes", false, "Accept confirmations without prompting")
|
||||
flags.BoolVarP(&a.options.yes, "yes", "y", false, "Accept confirmations without prompting")
|
||||
flags.BoolVar(&a.options.debug, "debug", false, "Log the method, URL, status, and duration of each HTTP request to stderr")
|
||||
root.PersistentPreRunE = func(cmd *cobra.Command, _ []string) error {
|
||||
if reason := cmd.Annotations["tea.text"]; reason != "" && a.options.json != "" {
|
||||
return clierr.Wrap(clierr.Usage, fmt.Errorf("%s; --json is not supported", reason))
|
||||
}
|
||||
if flag := cmd.Flags().Lookup("web"); flag != nil && flag.Value.String() == "true" && (a.options.json != "" || a.options.jq != "") {
|
||||
return clierr.Wrap(clierr.Usage, errors.New("--web cannot be combined with --json or --jq"))
|
||||
}
|
||||
if a.options.jq != "" && a.options.json == "" {
|
||||
return clierr.Wrap(clierr.Usage, errors.New("--jq requires --json"))
|
||||
if err := a.checkOutputFlags(cmd); err != nil {
|
||||
return err
|
||||
}
|
||||
if cmd.Flags().Changed("sudo") {
|
||||
if a.options.sudo == "" || strings.IndexFunc(a.options.sudo, func(r rune) bool { return unicode.IsSpace(r) || unicode.IsControl(r) }) >= 0 {
|
||||
@@ -136,9 +130,6 @@ func NewRoot(deps Dependencies) *cobra.Command {
|
||||
}
|
||||
}
|
||||
}
|
||||
if err := terminal.ValidateOutput(a.options.json, a.options.jq, commandOutputFields(cmd)); err != nil {
|
||||
return clierr.Wrap(clierr.Usage, err)
|
||||
}
|
||||
if offline(cmd) {
|
||||
return nil
|
||||
}
|
||||
@@ -161,6 +152,28 @@ func NewRoot(deps Dependencies) *cobra.Command {
|
||||
return root
|
||||
}
|
||||
|
||||
// checkOutputFlags validates --json and --jq before a command runs. --jq alone selects every field,
|
||||
// except on commands that filter their raw JSON themselves (annotation tea.rawjson).
|
||||
func (a *application) checkOutputFlags(cmd *cobra.Command) error {
|
||||
selected := ""
|
||||
switch {
|
||||
case cmd.Flags().Changed("json"):
|
||||
if a.options.json == "" {
|
||||
return clierr.Wrap(clierr.Usage, errors.New("--json= needs field names; use --json alone for every field"))
|
||||
}
|
||||
selected = "--json"
|
||||
case a.options.jq != "" && cmd.Annotations["tea.rawjson"] == "":
|
||||
selected, a.options.json = "--jq", "*"
|
||||
}
|
||||
if reason := cmd.Annotations["tea.text"]; reason != "" && selected != "" {
|
||||
return clierr.Wrap(clierr.Usage, fmt.Errorf("%s; %s is not supported", reason, selected))
|
||||
}
|
||||
if flag := cmd.Flags().Lookup("web"); flag != nil && flag.Value.String() == "true" && (a.options.json != "" || a.options.jq != "") {
|
||||
return clierr.Wrap(clierr.Usage, errors.New("--web cannot be combined with --json or --jq"))
|
||||
}
|
||||
return clierr.Wrap(clierr.Usage, terminal.ValidateOutput(a.options.json, a.options.jq, commandOutputFields(cmd)))
|
||||
}
|
||||
|
||||
// offline reports commands that must work without valid settings.
|
||||
func offline(cmd *cobra.Command) bool {
|
||||
if cmd.Annotations["tea.group"] == "true" {
|
||||
@@ -257,11 +270,13 @@ func Execute(ctx context.Context, deps Dependencies, args []string) error {
|
||||
return err
|
||||
}
|
||||
|
||||
var jsonFieldList = regexp.MustCompile(`^[a-z_]+(,[a-z_]+)*$`)
|
||||
var jsonFieldList = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_]*(,[A-Za-z][A-Za-z0-9_]*)*$`)
|
||||
|
||||
// joinJSONFields accepts `--json number,title` like `--json=number,title` unless the selected command
|
||||
// does not output those fields, in which case the word stays a positional argument. A bare --json
|
||||
// still selects every field. Unknown commands, such as aliases before expansion, are joined.
|
||||
// joinJSONFields accepts `--json number,title` like `--json=number,title`. A field-shaped word after a
|
||||
// bare --json stays a positional argument only when the selected command does not output those fields
|
||||
// and accepts the word as an argument, so a misspelled field reports the available fields rather than
|
||||
// an argument count. A bare --json still selects every field. Unknown commands, such as aliases before
|
||||
// expansion, are joined.
|
||||
func joinJSONFields(root *cobra.Command, args []string) []string {
|
||||
for i := 0; i < len(args); i++ {
|
||||
if args[i] == "--" {
|
||||
@@ -271,7 +286,7 @@ func joinJSONFields(root *cobra.Command, args []string) []string {
|
||||
continue
|
||||
}
|
||||
cmd, _, err := root.Find(slices.Concat(args[:i+1], args[i+2:]))
|
||||
if err == nil && terminal.ValidateOutput(args[i+1], "", commandOutputFields(cmd)) != nil {
|
||||
if err == nil && terminal.ValidateOutput(args[i+1], "", commandOutputFields(cmd)) != nil && acceptsArgs(args) {
|
||||
continue
|
||||
}
|
||||
args = slices.Concat(args[:i], []string{"--json=" + args[i+1]}, args[i+2:])
|
||||
@@ -279,6 +294,13 @@ func joinJSONFields(root *cobra.Command, args []string) []string {
|
||||
return args
|
||||
}
|
||||
|
||||
// acceptsArgs reports whether the command that args select accepts their positional arguments. It
|
||||
// parses a separate command tree, because parsing sets the flags of the one that runs.
|
||||
func acceptsArgs(args []string) bool {
|
||||
cmd, rest, err := NewRoot(Dependencies{}).Find(args)
|
||||
return err == nil && cmd.ParseFlags(rest) == nil && cmd.ValidateArgs(cmd.Flags().Args()) == nil
|
||||
}
|
||||
|
||||
// confirm asks for confirmation under the prompt policy unless --yes was supplied.
|
||||
func (a *application) confirm(cmd *cobra.Command, description string) error {
|
||||
s, err := a.deps.Settings.Load()
|
||||
@@ -364,6 +386,12 @@ func commandIndex(root *cobra.Command, args []string) int {
|
||||
return len(args)
|
||||
}
|
||||
|
||||
// configKeys are the preferences, instance settings and account settings that config get reads.
|
||||
var configKeys = []string{
|
||||
"browser", "color", "default_host", "editor", "git_protocol", "insecure", "oauth_client_id", "pager", "prompt",
|
||||
"remote", "ssh_agent", "ssh_certificate_principal", "ssh_host", "ssh_key", "ssh_key_fingerprint", "version",
|
||||
}
|
||||
|
||||
func commandOutputFields(cmd *cobra.Command) []string {
|
||||
if cmd.Name() == "status" && cmd.Parent() != nil {
|
||||
if cmd.Parent().Name() == "pr" {
|
||||
@@ -389,11 +417,7 @@ func commandOutputFields(cmd *cobra.Command) []string {
|
||||
}
|
||||
case "config":
|
||||
if cmd.Name() == "get" || cmd.Name() == "list" {
|
||||
return []string{
|
||||
"version", "default_host", "git_protocol", "editor", "prompt", "pager", "browser", "color", "remote",
|
||||
"ssh_host", "insecure", "oauth_client_id",
|
||||
"ssh_key", "ssh_agent", "ssh_certificate_principal", "ssh_key_fingerprint",
|
||||
}
|
||||
return configKeys
|
||||
}
|
||||
case "tea":
|
||||
if cmd.Name() == "doctor" {
|
||||
@@ -483,7 +507,7 @@ func commandOutputFields(cmd *cobra.Command) []string {
|
||||
return nil
|
||||
}
|
||||
|
||||
// jsonFields lists the JSON names of T's fields, which --json can select.
|
||||
// jsonFields lists the JSON names of T's fields, which --json can select, in alphabetical order.
|
||||
func jsonFields[T any]() []string {
|
||||
t := reflect.TypeFor[T]()
|
||||
fields := make([]string, 0, t.NumField())
|
||||
@@ -492,5 +516,6 @@ func jsonFields[T any]() []string {
|
||||
fields = append(fields, name)
|
||||
}
|
||||
}
|
||||
slices.Sort(fields)
|
||||
return fields
|
||||
}
|
||||
@@ -5,6 +5,7 @@ package app
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
@@ -12,6 +13,7 @@ import (
|
||||
"testing"
|
||||
|
||||
"gitea.dev/tea/internal/clierr"
|
||||
"github.com/spf13/cobra"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
@@ -60,8 +62,8 @@ func TestCommandLineEdgeCases(t *testing.T) {
|
||||
require.Equal(t, clierr.Usage, clierr.ExitCode(err))
|
||||
require.ErrorContains(t, err, "did you mean issue?")
|
||||
// A help flag supplied as a flag value must not bypass validation.
|
||||
err = Execute(context.Background(), deps, []string{"config", "list", "--jq", ".", "--remote", "-h"})
|
||||
require.ErrorContains(t, err, "--jq requires --json")
|
||||
err = Execute(context.Background(), deps, []string{"config", "list", "--jq", ".[", "--remote", "-h"})
|
||||
require.ErrorContains(t, err, "invalid --jq expression")
|
||||
require.NoError(t, Execute(context.Background(), deps, []string{"--version"}))
|
||||
require.NotContains(t, out.String(), "tea version")
|
||||
}
|
||||
@@ -88,12 +90,17 @@ func TestJoinJSONFields(t *testing.T) {
|
||||
{[]string{"repo", "clone", "--json", "alice/project"}, []string{"repo", "clone", "--json", "alice/project"}},
|
||||
{[]string{"issue", "list", "--json", "--state", "open"}, []string{"issue", "list", "--json", "--state", "open"}},
|
||||
{[]string{"search", "issues", "--", "--json", "id"}, []string{"search", "issues", "--", "--json", "id"}},
|
||||
{[]string{"issue", "list", "--json", "Number"}, []string{"issue", "list", "--json", "Number"}},
|
||||
{[]string{"search", "repo", "--json", "tea"}, []string{"search", "repo", "--json", "tea"}},
|
||||
{[]string{"search", "issue", "--json", "bug"}, []string{"search", "issue", "--json", "bug"}},
|
||||
{[]string{"repo", "clone", "--json", "project"}, []string{"repo", "clone", "--json", "project"}},
|
||||
{[]string{"search", "repo", "--json", "name,url", "tea"}, []string{"search", "repo", "--json=name,url", "tea"}},
|
||||
{[]string{"--json", "number", "my-alias"}, []string{"--json=number", "my-alias"}},
|
||||
// A word that cannot be an argument is a misspelled field, which then lists the available ones.
|
||||
{[]string{"issue", "list", "--json", "Number"}, []string{"issue", "list", "--json=Number"}},
|
||||
{[]string{"issue", "list", "--json", "number,titel"}, []string{"issue", "list", "--json=number,titel"}},
|
||||
{[]string{"run", "list", "--json", "workflow", "--limit", "5"}, []string{"run", "list", "--json=workflow", "--limit", "5"}},
|
||||
{[]string{"issue", "view", "--json", "titel"}, []string{"issue", "view", "--json", "titel"}},
|
||||
{[]string{"issue", "view", "5", "--json", "titel"}, []string{"issue", "view", "5", "--json=titel"}},
|
||||
} {
|
||||
deps, _, _ := testDeps(t)
|
||||
require.Equal(t, tc.want, joinJSONFields(NewRoot(deps), tc.args), tc.args)
|
||||
@@ -148,3 +155,40 @@ func TestSudoSendsHeaderAfterIdentityCheck(t *testing.T) {
|
||||
requireUsageBeforeRequests(t, "--sudo cannot be used with auth commands", "auth", "status", "--sudo", "bob")
|
||||
requireUsageBeforeRequests(t, "--sudo requires a username", "repo", "view", "--sudo", "")
|
||||
}
|
||||
|
||||
func TestJSONFieldErrorsListAvailableFields(t *testing.T) {
|
||||
requireUsageBeforeRequests(t, "unknown JSON field \"titel\"\navailable fields: assignees, author, body", "issue", "list", "--json", "number,titel")
|
||||
requireUsageBeforeRequests(t, "unknown JSON field \"workflow\"\navailable fields: actor, attempt, branch", "run", "list", "--json", "workflow")
|
||||
requireUsageBeforeRequests(t, "--json= needs field names; use --json alone for every field", "pr", "list", "--json=")
|
||||
requireUsageBeforeRequests(t, "version prints text; --jq is not supported", "version", "--jq", ".")
|
||||
}
|
||||
|
||||
func TestJQImpliesJSONAndShorthands(t *testing.T) {
|
||||
server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
_, _ = w.Write([]byte(`[{"id":1,"username":"team","full_name":"Team"}]`))
|
||||
}))
|
||||
defer server.Close()
|
||||
for _, args := range [][]string{{"org", "list", "--jq", ".[].name"}, {"org", "list", "-q", ".[].name"}, {"-q", ".[0].full_name", "org", "list"}} {
|
||||
deps, out, _ := serverDeps(t, server.URL)
|
||||
require.NoError(t, Execute(context.Background(), deps, args), args)
|
||||
require.Contains(t, []string{"team\n", "Team\n"}, out.String(), args)
|
||||
}
|
||||
deps, _, _ := testDeps(t)
|
||||
flags := NewRoot(deps).PersistentFlags()
|
||||
require.Equal(t, "yes", flags.ShorthandLookup("y").Name)
|
||||
require.Equal(t, "jq", flags.ShorthandLookup("q").Name)
|
||||
}
|
||||
|
||||
func TestRawJSONCommandsKeepJQToThemselves(t *testing.T) {
|
||||
deps, out, _ := testDeps(t)
|
||||
root := NewRoot(deps)
|
||||
root.AddCommand(&cobra.Command{Use: "raw", Annotations: map[string]string{"tea.text": "raw prints text", "tea.rawjson": "true"}, RunE: func(cmd *cobra.Command, _ []string) error {
|
||||
_, err := fmt.Fprintf(cmd.OutOrStdout(), "json=%q", cmd.Flag("json").Value)
|
||||
return err
|
||||
}})
|
||||
root.SetArgs([]string{"raw", "--jq", ".x"})
|
||||
require.NoError(t, root.Execute())
|
||||
require.Equal(t, `json=""`, out.String())
|
||||
root.SetArgs([]string{"raw", "--json"})
|
||||
require.ErrorContains(t, root.Execute(), "raw prints text; --json is not supported")
|
||||
}
|
||||
Reference in new issue
Block a user