From 1a4862e96feb230e8536fc5949b0b6ce14b51107 Mon Sep 17 00:00:00 2001 From: Andrey Markelov Date: Mon, 22 Jun 2026 14:28:55 -0700 Subject: [PATCH] Add --output=json support to ls command Refactor ls to separate entry preparation from rendering. JSON output emits an entries array with structured metadata. Deleted entries use a boolean field instead of <<>> text decoration. Also fix outputJSONRequested to use last-flag-wins semantics matching Cobra behavior. --- README.md | 29 ++++- cmd/ls.go | 148 ++++++++++++++++++----- cmd/ls_test.go | 287 +++++++++++++++++++++++++++++++++++++++++++++ cmd/output.go | 22 +++- cmd/output_test.go | 35 ++++++ 5 files changed, 488 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index 72e9445..167de2d 100644 --- a/README.md +++ b/README.md @@ -138,12 +138,13 @@ Text output is the default. JSON output is available through the global `--outpu ```sh $ dbxcli --output=json +$ dbxcli ls --output=json / $ dbxcli mkdir --output=json /new-folder $ dbxcli rm --output=json /old-file.txt $ dbxcli restore --output=json /Reports/old.pdf 015f... ``` -JSON support is rolling out command by command. Commands that have not been migrated return a JSON error whose `error.message` is `structured output is not supported for this command yet` when used with `--output=json`. +JSON support is rolling out command by command. Currently migrated commands are `ls`, `mkdir`, `rm`, and `restore`. Commands that have not been migrated return a JSON error whose `error.message` is `structured output is not supported for this command yet` when used with `--output=json`. Command results are written to stdout. Status, progress, warnings, diagnostics, and verbose logs are written to stderr. @@ -189,6 +190,32 @@ Commands that operate on multiple paths return a `results` array: } ``` +List commands such as `ls` return an `input` object and an `entries` array: + +```json +{ + "input": { + "path": "/Reports", + "recursive": false, + "include_deleted": false, + "only_deleted": false, + "long": false, + "reverse": false, + "time": "server" + }, + "entries": [ + { + "type": "file", + "path_display": "/Reports/q1.pdf", + "path_lower": "/reports/q1.pdf", + "id": "id:...", + "rev": "...", + "size": 123 + } + ] +} +``` + In JSON mode, command errors are also written to stdout. The process still exits with a non-zero status: ```json diff --git a/cmd/ls.go b/cmd/ls.go index 2c1bc4d..ee71bf3 100644 --- a/cmd/ls.go +++ b/cmd/ls.go @@ -18,9 +18,10 @@ import ( "errors" "fmt" "io" - "os" + "strings" "text/tabwriter" + "github.com/dropbox/dbxcli/internal/output" "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/files" "github.com/dustin/go-humanize" "github.com/spf13/cobra" @@ -28,6 +29,23 @@ import ( const deletedItemFormatString = "<<%s>>" +type lsInput struct { + Path string `json:"path"` + Recursive bool `json:"recursive"` + IncludeDeleted bool `json:"include_deleted"` + OnlyDeleted bool `json:"only_deleted"` + Long bool `json:"long"` + Sort string `json:"sort,omitempty"` + Reverse bool `json:"reverse"` + Time string `json:"time,omitempty"` + TimeFormat string `json:"time_format,omitempty"` +} + +type lsOutput struct { + Input lsInput `json:"input"` + Entries []jsonMetadata `json:"entries"` +} + // Sends a get_metadata request for a given path and returns the response func getFileMetadata(c files.Client, path string) (files.IsMetadata, error) { arg := files.NewGetMetadataArg(path) @@ -118,23 +136,9 @@ func ls(cmd *cobra.Command, args []string) (err error) { arg.IncludeDeleted = arg.IncludeDeleted || onlyDeleted opts := parseLsOptions(cmd) - w := new(tabwriter.Writer) - w.Init(os.Stdout, 4, 8, 1, ' ', 0) - itemCounter := 0 - printItem := func(message string) { - itemCounter = itemCounter + 1 - _, _ = fmt.Fprint(w, message) - if (itemCounter%4 == 0) || opts.long { - _, _ = fmt.Fprintln(w) - } - } - - dbx := files.New(config) - - if opts.long { - _, _ = fmt.Fprint(w, "Revision\tSize\tLast modified\tPath\n") - } + dbx := filesNewFunc(config) + var entries []files.IsMetadata if path != "" { var metaRes files.IsMetadata metaRes, err = getFileMetadata(dbx, path) @@ -145,15 +149,14 @@ func ls(cmd *cobra.Command, args []string) (err error) { switch f := metaRes.(type) { case *files.FileMetadata: if !onlyDeleted { - printItem(formatFileMetadataWithOpts(f, opts)) - return finishListOutput(w, itemCounter, opts) + entries = []files.IsMetadata{f} + return renderLsOutput(cmd, path, arg, onlyDeleted, opts, entries) } } } res, err := dbx.ListFolder(arg) - var entries []files.IsMetadata if err != nil { if !isListFolderNotFolderError(err) { return err @@ -179,7 +182,16 @@ func ls(cmd *cobra.Command, args []string) (err error) { } sortEntries(entries, opts) + entries, err = prepareLsEntries(dbx, entries, onlyDeleted) + if err != nil { + return err + } + return renderLsOutput(cmd, path, arg, onlyDeleted, opts, entries) +} + +func prepareLsEntries(dbx files.Client, entries []files.IsMetadata, onlyDeleted bool) ([]files.IsMetadata, error) { + var filtered []files.IsMetadata for _, entry := range entries { deletedItem, isDeleted := entry.(*files.DeletedMetadata) if isDeleted { @@ -191,7 +203,7 @@ func ls(cmd *cobra.Command, args []string) (err error) { // get_metadata request for the same path and using that response instead. revision, err := getFileMetadata(dbx, deletedItem.PathLower) if err != nil { - return err + return nil, err } entry = revision } @@ -203,15 +215,96 @@ func ls(cmd *cobra.Command, args []string) (err error) { } setPathDisplayAsDeleted(entry) } - switch f := entry.(type) { - case *files.FileMetadata: + switch entry.(type) { + case *files.FileMetadata, *files.FolderMetadata: if !onlyDeleted { - printItem(formatFileMetadataWithOpts(f, opts)) + filtered = append(filtered, entry) } + case *files.DeletedMetadata: + filtered = append(filtered, entry) + } + } + return filtered, nil +} + +func renderLsOutput(cmd *cobra.Command, path string, arg *files.ListFolderArg, onlyDeleted bool, opts listOptions, entries []files.IsMetadata) error { + out := commandOutput(cmd) + if commandOutputFormat(cmd) != output.FormatJSON { + return out.RenderText(func(w io.Writer) error { + return renderLsResults(w, entries, opts) + }) + } + + return out.Render(nil, lsOutput{ + Input: newLsInput(path, arg, onlyDeleted, opts), + Entries: jsonMetadataListFromLsEntries(entries), + }) +} + +func newLsInput(path string, arg *files.ListFolderArg, onlyDeleted bool, opts listOptions) lsInput { + displayPath := path + if displayPath == "" { + displayPath = "/" + } + return lsInput{ + Path: displayPath, + Recursive: arg.Recursive, + IncludeDeleted: arg.IncludeDeleted, + OnlyDeleted: onlyDeleted, + Long: opts.long, + Sort: opts.sortBy, + Reverse: opts.reverse, + Time: opts.timeField, + TimeFormat: opts.timeFormat, + } +} + +func jsonMetadataListFromLsEntries(entries []files.IsMetadata) []jsonMetadata { + result := make([]jsonMetadata, 0, len(entries)) + for _, entry := range entries { + result = append(result, jsonMetadataFromLsEntry(entry)) + } + return result +} + +func jsonMetadataFromLsEntry(entry files.IsMetadata) jsonMetadata { + result := jsonMetadataFromDropbox(entry) + if path, ok := undecoratedDeletedPath(result.PathDisplay); ok { + result.PathDisplay = path + result.Deleted = true + } + return result +} + +func undecoratedDeletedPath(path string) (string, bool) { + if strings.HasPrefix(path, "<<") && strings.HasSuffix(path, ">>") { + return strings.TrimSuffix(strings.TrimPrefix(path, "<<"), ">>"), true + } + return path, false +} + +func renderLsResults(out io.Writer, entries []files.IsMetadata, opts listOptions) error { + w := new(tabwriter.Writer) + w.Init(out, 4, 8, 1, ' ', 0) + itemCounter := 0 + printItem := func(message string) { + itemCounter = itemCounter + 1 + _, _ = fmt.Fprint(w, message) + if (itemCounter%4 == 0) || opts.long { + _, _ = fmt.Fprintln(w) + } + } + + if opts.long { + _, _ = fmt.Fprint(w, "Revision\tSize\tLast modified\tPath\n") + } + + for _, entry := range entries { + switch f := entry.(type) { + case *files.FileMetadata: + printItem(formatFileMetadataWithOpts(f, opts)) case *files.FolderMetadata: - if !onlyDeleted { - printItem(formatFolderMetadata(f, opts.long)) - } + printItem(formatFolderMetadata(f, opts.long)) case *files.DeletedMetadata: printItem(formatDeletedMetadata(f, opts.long)) } @@ -265,4 +358,5 @@ func init() { lsCmd.Flags().BoolP("reverse", "r", false, "Reverse sort order") lsCmd.Flags().String("time", "server", "Time field: server, client") lsCmd.Flags().String("time-format", "", "Time format: short (2006-01-02 15:04), rfc3339") + enableStructuredOutput(lsCmd) } diff --git a/cmd/ls_test.go b/cmd/ls_test.go index 1642565..21c04b1 100644 --- a/cmd/ls_test.go +++ b/cmd/ls_test.go @@ -1,6 +1,8 @@ package cmd import ( + "bytes" + "encoding/json" "fmt" "strings" "testing" @@ -9,6 +11,7 @@ import ( "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox" "github.com/dropbox/dropbox-sdk-go-unofficial/v6/dropbox/files" + "github.com/spf13/cobra" ) func TestFormatFolderMetadata(t *testing.T) { @@ -150,6 +153,250 @@ func TestFinishListOutputAddsTrailingNewlineForPartialShortRows(t *testing.T) { } } +func TestRenderLsResultsShortModeUsesFourColumns(t *testing.T) { + entries := []files.IsMetadata{ + &files.FolderMetadata{Metadata: files.Metadata{PathDisplay: "/one"}}, + &files.FolderMetadata{Metadata: files.Metadata{PathDisplay: "/two"}}, + &files.FolderMetadata{Metadata: files.Metadata{PathDisplay: "/three"}}, + &files.FolderMetadata{Metadata: files.Metadata{PathDisplay: "/four"}}, + &files.FolderMetadata{Metadata: files.Metadata{PathDisplay: "/five"}}, + } + + var out bytes.Buffer + if err := renderLsResults(&out, entries, listOptions{}); err != nil { + t.Fatalf("renderLsResults returned error: %v", err) + } + + lines := strings.Split(strings.TrimSuffix(out.String(), "\n"), "\n") + if len(lines) != 2 { + t.Fatalf("output = %q, want two short-mode rows", out.String()) + } + if !strings.Contains(lines[0], "/one") || !strings.Contains(lines[0], "/four") { + t.Fatalf("first row = %q, want first four entries", lines[0]) + } + if !strings.Contains(lines[1], "/five") { + t.Fatalf("second row = %q, want fifth entry", lines[1]) + } +} + +func TestLsJSONListsEntriesAndInput(t *testing.T) { + cmd, stdout := testLsCmd(t) + setLsOutputJSON(t, cmd) + setLsFlag(t, cmd, "recurse", "true") + setLsFlag(t, cmd, "include-deleted", "true") + setLsFlag(t, cmd, "long", "true") + setLsFlag(t, cmd, "sort", "name") + setLsFlag(t, cmd, "reverse", "true") + setLsFlag(t, cmd, "time", "client") + setLsFlag(t, cmd, "time-format", "rfc3339") + + var listArg *files.ListFolderArg + mock := &mockFilesClient{ + listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) { + listArg = arg + return &files.ListFolderResult{ + Entries: []files.IsMetadata{ + &files.FileMetadata{ + Metadata: files.Metadata{ + Name: "file.txt", + PathDisplay: "/file.txt", + PathLower: "/file.txt", + }, + Id: "id:file", + Rev: "rev-file", + Size: 42, + }, + &files.FolderMetadata{ + Metadata: files.Metadata{ + Name: "Folder", + PathDisplay: "/Folder", + PathLower: "/folder", + }, + Id: "id:folder", + }, + }, + HasMore: false, + }, nil + }, + } + stubFilesClient(t, mock) + + if err := ls(cmd, nil); err != nil { + t.Fatalf("ls error: %v", err) + } + if listArg == nil { + t.Fatal("ListFolder was not called") + } + if listArg.Path != "" { + t.Fatalf("ListFolder path = %q, want root empty path", listArg.Path) + } + if !listArg.Recursive || !listArg.IncludeDeleted { + t.Fatalf("ListFolder flags = recursive:%v include_deleted:%v, want true/true", listArg.Recursive, listArg.IncludeDeleted) + } + + got := decodeLsOutput(t, stdout) + if got.Input.Path != "/" { + t.Fatalf("input path = %q, want /", got.Input.Path) + } + if !got.Input.Recursive || !got.Input.IncludeDeleted || got.Input.OnlyDeleted || !got.Input.Long { + t.Fatalf("input flags = %+v, want recursive/include_deleted/long true and only_deleted false", got.Input) + } + if got.Input.Sort != "name" || !got.Input.Reverse || got.Input.Time != "client" || got.Input.TimeFormat != "rfc3339" { + t.Fatalf("input options = %+v, want sort/name reverse/client/rfc3339", got.Input) + } + if len(got.Entries) != 2 { + t.Fatalf("entries = %d, want 2", len(got.Entries)) + } + if got.Entries[0].Type != "folder" || got.Entries[0].PathDisplay != "/Folder" { + t.Fatalf("first entry = %#v, want sorted folder", got.Entries[0]) + } + if got.Entries[1].Type != "file" || got.Entries[1].Rev != "rev-file" || got.Entries[1].Size == nil || *got.Entries[1].Size != 42 { + t.Fatalf("second entry = %#v, want file metadata", got.Entries[1]) + } +} + +func TestLsJSONFilePathUsesMetadata(t *testing.T) { + cmd, stdout := testLsCmd(t) + setLsOutputJSON(t, cmd) + var metadataPath string + + mock := &mockFilesClient{ + getMetadataFn: func(arg *files.GetMetadataArg) (files.IsMetadata, error) { + metadataPath = arg.Path + return &files.FileMetadata{ + Metadata: files.Metadata{ + PathDisplay: "/file.txt", + PathLower: "/file.txt", + }, + Id: "id:file", + Rev: "rev-file", + Size: 7, + }, nil + }, + listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) { + t.Fatalf("ListFolder called for file path: %v", arg) + return nil, nil + }, + } + stubFilesClient(t, mock) + + if err := ls(cmd, []string{"/file.txt"}); err != nil { + t.Fatalf("ls error: %v", err) + } + if metadataPath != "/file.txt" { + t.Fatalf("metadata path = %q, want /file.txt", metadataPath) + } + got := decodeLsOutput(t, stdout) + if got.Input.Path != "/file.txt" { + t.Fatalf("input path = %q, want /file.txt", got.Input.Path) + } + if len(got.Entries) != 1 || got.Entries[0].Type != "file" { + t.Fatalf("entries = %#v, want one file", got.Entries) + } +} + +func TestLsJSONDeletedEntryIsStructured(t *testing.T) { + cmd, stdout := testLsCmd(t) + setLsOutputJSON(t, cmd) + setLsFlag(t, cmd, "include-deleted", "true") + + mock := &mockFilesClient{ + listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) { + return &files.ListFolderResult{ + Entries: []files.IsMetadata{ + &files.DeletedMetadata{ + Metadata: files.Metadata{ + PathDisplay: "/removed.txt", + PathLower: "/removed.txt", + }, + }, + }, + HasMore: false, + }, nil + }, + listRevisionsFn: func(arg *files.ListRevisionsArg) (*files.ListRevisionsResult, error) { + if arg.Path != "/removed.txt" { + t.Fatalf("ListRevisions path = %q, want /removed.txt", arg.Path) + } + return files.NewListRevisionsResult(false, []*files.FileMetadata{ + { + Metadata: files.Metadata{ + PathDisplay: "/removed.txt", + PathLower: "/removed.txt", + }, + Rev: "rev-removed", + Size: 9, + }, + }), nil + }, + } + stubFilesClient(t, mock) + + if err := ls(cmd, nil); err != nil { + t.Fatalf("ls error: %v", err) + } + got := decodeLsOutput(t, stdout) + if len(got.Entries) != 1 { + t.Fatalf("entries = %d, want 1", len(got.Entries)) + } + entry := got.Entries[0] + if entry.PathDisplay != "/removed.txt" { + t.Fatalf("path_display = %q, want undecorated path", entry.PathDisplay) + } + if !entry.Deleted { + t.Fatal("deleted = false, want true") + } + if strings.Contains(stdout.String(), "<<") { + t.Fatalf("JSON output = %s, want no text deleted marker", stdout.String()) + } +} + +func TestLsTextUsesCommandOutput(t *testing.T) { + cmd, stdout := testLsCmd(t) + mock := &mockFilesClient{ + listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) { + return &files.ListFolderResult{ + Entries: []files.IsMetadata{ + &files.FolderMetadata{Metadata: files.Metadata{PathDisplay: "/docs"}}, + }, + HasMore: false, + }, nil + }, + } + stubFilesClient(t, mock) + + if err := ls(cmd, nil); err != nil { + t.Fatalf("ls error: %v", err) + } + if got := stdout.String(); !strings.Contains(got, "/docs") { + t.Fatalf("stdout = %q, want command output to contain /docs", got) + } +} + +func TestLsJSONErrorWritesNoOutput(t *testing.T) { + cmd, stdout := testLsCmd(t) + setLsOutputJSON(t, cmd) + mock := &mockFilesClient{ + listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) { + return nil, fmt.Errorf("list failed") + }, + } + stubFilesClient(t, mock) + + if err := ls(cmd, nil); err == nil { + t.Fatal("expected ls error") + } + if got := stdout.String(); got != "" { + t.Fatalf("stdout = %q, want empty output on error", got) + } +} + +func TestLsCommandSupportsStructuredOutput(t *testing.T) { + if !commandSupportsStructuredOutput(lsCmd) { + t.Fatal("ls command should support structured output") + } +} + func TestIsListFolderNotFolderErrorHandlesWrappedErrors(t *testing.T) { apiErr := files.ListFolderAPIError{ EndpointError: &files.ListFolderError{ @@ -203,3 +450,43 @@ func TestDropboxConfigConstruction(t *testing.T) { t.Error("config token not set") } } + +func testLsCmd(t *testing.T) (*cobra.Command, *bytes.Buffer) { + t.Helper() + + var stdout bytes.Buffer + cmd := &cobra.Command{Use: "ls"} + cmd.SetOut(&stdout) + cmd.Flags().BoolP("long", "l", false, "") + cmd.Flags().BoolP("recurse", "R", false, "") + cmd.Flags().BoolP("include-deleted", "d", false, "") + cmd.Flags().BoolP("only-deleted", "D", false, "") + cmd.Flags().String("sort", "", "") + cmd.Flags().BoolP("reverse", "r", false, "") + cmd.Flags().String("time", "server", "") + cmd.Flags().String("time-format", "", "") + cmd.Flags().String(outputFlag, "text", "") + return cmd, &stdout +} + +func setLsOutputJSON(t *testing.T, cmd *cobra.Command) { + t.Helper() + setLsFlag(t, cmd, outputFlag, "json") +} + +func setLsFlag(t *testing.T, cmd *cobra.Command, name, value string) { + t.Helper() + if err := cmd.Flags().Set(name, value); err != nil { + t.Fatalf("set %s: %v", name, err) + } +} + +func decodeLsOutput(t *testing.T, out *bytes.Buffer) lsOutput { + t.Helper() + + var got lsOutput + if err := json.NewDecoder(out).Decode(&got); err != nil { + t.Fatalf("decode JSON output: %v\noutput: %s", err, out.String()) + } + return got +} diff --git a/cmd/output.go b/cmd/output.go index 84c9ffd..1be2cfc 100644 --- a/cmd/output.go +++ b/cmd/output.go @@ -154,17 +154,29 @@ func renderCommandErrorWithJSON(cmd *cobra.Command, err error, forceJSON bool) { // outputJSONRequested is a narrow pre-parse fallback for errors that happen // before Cobra has resolved a command/flag context, such as unknown commands. func outputJSONRequested(args []string) bool { + jsonRequested := false for i := 0; i < len(args); i++ { - switch args[i] { + arg := args[i] + switch arg { case "--": - return false + return jsonRequested case "--output=json": - return true + jsonRequested = true + case "--output=text": + jsonRequested = false case "--output": - return i+1 < len(args) && args[i+1] == "json" + if i+1 >= len(args) { + return false + } + jsonRequested = args[i+1] == "json" + i++ + default: + if strings.HasPrefix(arg, "--output=") { + jsonRequested = strings.TrimPrefix(arg, "--output=") == "json" + } } } - return false + return jsonRequested } // jsonErrorCode derives stable script-facing codes from existing CLI errors. diff --git a/cmd/output_test.go b/cmd/output_test.go index 07fc40c..9ddb6e8 100644 --- a/cmd/output_test.go +++ b/cmd/output_test.go @@ -355,11 +355,46 @@ func TestOutputJSONRequested(t *testing.T) { args: []string{"--output", "yaml", "missing"}, want: false, }, + { + name: "invalid format before json", + args: []string{"--output", "yaml", "--output", "json", "missing"}, + want: true, + }, + { + name: "invalid format after json", + args: []string{"--output", "json", "--output", "yaml", "missing"}, + want: false, + }, + { + name: "last separate flag wins text", + args: []string{"--output", "json", "--output", "text", "missing"}, + want: false, + }, + { + name: "last separate flag wins json", + args: []string{"--output", "text", "--output", "json", "missing"}, + want: true, + }, + { + name: "last equals flag wins text", + args: []string{"--output=json", "--output=text", "missing"}, + want: false, + }, + { + name: "last equals flag wins json", + args: []string{"--output=text", "--output=json", "missing"}, + want: true, + }, { name: "after double dash", args: []string{"mkdir", "--", "--output=json"}, want: false, }, + { + name: "output before double dash", + args: []string{"--output=json", "mkdir", "--", "--output=text"}, + want: true, + }, } for _, tt := range tests {