diff --git a/internal/app/app.go b/internal/app/app.go index 9e848b66..767b9a20 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -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 } diff --git a/internal/app/app_test.go b/internal/app/app_test.go index fe163cd6..831e51be 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -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") +} diff --git a/internal/terminal/output.go b/internal/terminal/output.go index 8beab7f1..7c74431c 100644 --- a/internal/terminal/output.go +++ b/internal/terminal/output.go @@ -4,17 +4,36 @@ package terminal import ( + "bytes" "encoding/json" "errors" "fmt" "io" + "slices" "strings" "github.com/itchyny/gojq" ) +// JSONOutput selects, filters, and formats command results as JSON. +type JSONOutput struct { + // Fields holds the comma-separated fields to keep; "" and "*" keep every field. + Fields string + // Query is a jq expression applied after field selection. + Query string + // Pretty indents the JSON for a terminal and strips control characters from raw jq strings. + Pretty bool + // Color highlights pretty JSON keys and values. + Color bool +} + // WriteJSON projects explicit result fields and applies an in-process jq filter. func WriteJSON(w io.Writer, value any, fields, query string) error { + return JSONOutput{Fields: fields, Query: query}.Write(w, value) +} + +// Write projects explicit result fields of value, applies the jq filter, and writes the results. +func (o JSONOutput) Write(w io.Writer, value any) error { data, err := json.Marshal(value) if err != nil { return err @@ -23,7 +42,7 @@ func WriteJSON(w io.Writer, value any, fields, query string) error { if err := json.Unmarshal(data, &normalized); err != nil { return err } - if names := fieldNames(fields); names != nil { + if names := fieldNames(o.Fields); names != nil { selectFields := func(item any) (any, error) { object, ok := item.(map[string]any) if !ok { @@ -54,12 +73,10 @@ func WriteJSON(w io.Writer, value any, fields, query string) error { } } } - encoder := json.NewEncoder(w) - encoder.SetEscapeHTML(false) - if query == "" { - return encoder.Encode(normalized) + if o.Query == "" { + return o.encode(w, normalized) } - parsed, err := parseQuery(query) + parsed, err := parseQuery(o.Query) if err != nil { return err } @@ -73,25 +90,95 @@ func WriteJSON(w io.Writer, value any, fields, query string) error { return fmt.Errorf("evaluate --jq: %w", err) } if str, ok := value.(string); ok { + if o.Pretty { + str = SafeText(str) + } if _, err := fmt.Fprintln(w, str); err != nil { return err } - } else if err := encoder.Encode(value); err != nil { + } else if err := o.encode(w, value); err != nil { return err } } } +// encode writes one JSON value followed by a newline. +func (o JSONOutput) encode(w io.Writer, value any) error { + var buf bytes.Buffer + encoder := json.NewEncoder(&buf) + encoder.SetEscapeHTML(false) + if o.Pretty { + encoder.SetIndent("", " ") + } + if err := encoder.Encode(value); err != nil { + return err + } + text := buf.String() + if o.Pretty { + // The encoder escapes control characters but not format characters such as bidirectional overrides. + text = SafeText(text) + } + if o.Color { + text = colorJSON(text) + } + _, err := io.WriteString(w, text) + return err +} + +// JSON colors, following gh: bold blue keys, green strings, yellow booleans, cyan null. +const ( + jsonKeyColor = "\x1b[1;34m" + jsonStringColor = "\x1b[32m" + jsonBoolColor = "\x1b[33m" + jsonNullColor = "\x1b[36m" + jsonResetColor = "\x1b[0m" +) + +// colorJSON highlights the keys and literal values of encoded JSON text. +func colorJSON(text string) string { + var b strings.Builder + for i := 0; i < len(text); { + color, end := "", i+1 + switch c := text[i]; { + case c == '"': + for end < len(text) && text[end] != '"' { + if text[end] == '\\' { + end++ + } + end++ + } + end++ + color = jsonStringColor + if strings.HasPrefix(strings.TrimLeft(text[end:], " "), ":") { + color = jsonKeyColor + } + case strings.HasPrefix(text[i:], "true"): + color, end = jsonBoolColor, i+len("true") + case strings.HasPrefix(text[i:], "false"): + color, end = jsonBoolColor, i+len("false") + case strings.HasPrefix(text[i:], "null"): + color, end = jsonNullColor, i+len("null") + } + if color == "" { + b.WriteString(text[i:end]) + } else { + b.WriteString(color + text[i:end] + jsonResetColor) + } + i = end + } + return b.String() +} + // ValidateOutput catches invalid projection/filter syntax before a service mutates data. +// An unknown field error lists the allowed fields. func ValidateOutput(fields, query string, allowed []string) error { if names := fieldNames(fields); names != nil && allowed != nil { - known := map[string]bool{} - for _, name := range allowed { - known[name] = true - } for _, name := range names { - if !known[name] { - return fmt.Errorf("unknown JSON field %q", name) + if name == "" { + return errors.New("--json field names cannot be empty; use --json alone for every field") + } + if !slices.Contains(allowed, name) { + return fmt.Errorf("unknown JSON field %q\navailable fields: %s", name, strings.Join(allowed, ", ")) } } } diff --git a/internal/terminal/output_test.go b/internal/terminal/output_test.go index 8d41a1a4..bf9fc114 100644 --- a/internal/terminal/output_test.go +++ b/internal/terminal/output_test.go @@ -20,5 +20,23 @@ func TestJSONFieldSelectionAndJQ(t *testing.T) { require.Equal(t, "[]\n", out.String()) require.Error(t, WriteJSON(&out, value, "unknown", "")) require.NoError(t, ValidateOutput("number, title", "", []string{"number", "title"})) - require.EqualError(t, ValidateOutput("number, foo", "", []string{"number"}), `unknown JSON field "foo"`) + require.EqualError(t, ValidateOutput("number, foo", "", []string{"number", "title"}), "unknown JSON field \"foo\"\navailable fields: number, title") + require.EqualError(t, ValidateOutput("number,,title", "", []string{"number", "title"}), "--json field names cannot be empty; use --json alone for every field") +} + +func TestJSONOutputIsIndentedAndColoredForTerminals(t *testing.T) { + var out bytes.Buffer + value := []map[string]any{{"title": "a \u202e\x1b[31mb", "done": true, "due": nil, "number": 1}} + require.NoError(t, JSONOutput{Fields: "number,title", Pretty: true}.Write(&out, value)) + require.Equal(t, "[\n {\n \"number\": 1,\n \"title\": \"a \\u001b[31mb\"\n }\n]\n", out.String()) + out.Reset() + require.NoError(t, JSONOutput{Pretty: true, Color: true}.Write(&out, map[string]any{"done": true, "due": nil, "title": "x: \"y\""})) + require.Equal(t, "{\n \x1b[1;34m\"done\"\x1b[0m: \x1b[33mtrue\x1b[0m,\n \x1b[1;34m\"due\"\x1b[0m: \x1b[36mnull\x1b[0m,\n \x1b[1;34m\"title\"\x1b[0m: \x1b[32m\"x: \\\"y\\\"\"\x1b[0m\n}\n", out.String()) + out.Reset() + // Raw jq strings on a terminal cannot carry escape sequences; piped they stay byte-exact. + require.NoError(t, JSONOutput{Query: ".title", Pretty: true}.Write(&out, value[0])) + require.Equal(t, "a [31mb\n", out.String()) + out.Reset() + require.NoError(t, WriteJSON(&out, value[0], "", ".title")) + require.Equal(t, "a \u202e\x1b[31mb\n", out.String()) }