Extract parseListOptions with validation and add --recursive flag

cmd/list_options.go

@@ -0,0 +1,82 @@
+package cmd
+
+import "github.com/spf13/cobra"
+
+func parseListOptions(cmd *cobra.Command) (listOptions, error) {
+	opts := listOptions{
+		long:       optionalBoolFlag(cmd, "long"),
+		timeField:  optionalStringFlag(cmd, "time"),
+		timeFormat: optionalStringFlag(cmd, "time-format"),
+		sortBy:     optionalStringFlag(cmd, "sort"),
+		reverse:    optionalBoolFlag(cmd, "reverse"),
+		limit:      optionalUint64Flag(cmd, "limit"),
+	}
+	if err := validateListOptions(opts); err != nil {
+		return listOptions{}, err
+	}
+	return opts, nil
+}
+
+func optionalBoolFlag(cmd *cobra.Command, name string) bool {
+	if cmd.Flags().Lookup(name) == nil {
+		return false
+	}
+	value, _ := cmd.Flags().GetBool(name)
+	return value
+}
+
+func optionalStringFlag(cmd *cobra.Command, name string) string {
+	if cmd.Flags().Lookup(name) == nil {
+		return ""
+	}
+	value, _ := cmd.Flags().GetString(name)
+	return value
+}
+
+func optionalUint64Flag(cmd *cobra.Command, name string) uint64 {
+	if cmd.Flags().Lookup(name) == nil {
+		return 0
+	}
+	value, _ := cmd.Flags().GetUint64(name)
+	return value
+}
+
+func validateListOptions(opts listOptions) error {
+	if !validListSort(opts.sortBy) {
+		return invalidArgumentsErrorfWithDetails("invalid --sort %q (use name, size, time, or type)", flagValueErrorDetails("sort", opts.sortBy), opts.sortBy)
+	}
+	if !validListTimeField(opts.timeField) {
+		return invalidArgumentsErrorfWithDetails("invalid --time %q (use server or client)", flagValueErrorDetails("time", opts.timeField), opts.timeField)
+	}
+	if !validListTimeFormat(opts.timeFormat) {
+		return invalidArgumentsErrorfWithDetails("invalid --time-format %q (use short or rfc3339)", flagValueErrorDetails("time-format", opts.timeFormat), opts.timeFormat)
+	}
+	return nil
+}
+
+func validListSort(sortBy string) bool {
+	switch sortBy {
+	case "", "name", "size", "time", "type":
+		return true
+	default:
+		return false
+	}
+}
+
+func validListTimeField(timeField string) bool {
+	switch timeField {
+	case "", "server", "client":
+		return true
+	default:
+		return false
+	}
+}
+
+func validListTimeFormat(timeFormat string) bool {
+	switch timeFormat {
+	case "", "short", "rfc3339":
+		return true
+	default:
+		return false
+	}
+}

cmd/ls.go

@@ -104,23 +104,6 @@ func setPathDisplayAsDeleted(metadata files.IsMetadata) {
 	}
 }
 
-func parseLsOptions(cmd *cobra.Command) listOptions {
-	long, _ := cmd.Flags().GetBool("long")
-	timeField, _ := cmd.Flags().GetString("time")
-	timeFormat, _ := cmd.Flags().GetString("time-format")
-	sortBy, _ := cmd.Flags().GetString("sort")
-	reverse, _ := cmd.Flags().GetBool("reverse")
-	limit, _ := cmd.Flags().GetUint64("limit")
-	return listOptions{
-		long:       long,
-		timeField:  timeField,
-		timeFormat: timeFormat,
-		sortBy:     sortBy,
-		reverse:    reverse,
-		limit:      limit,
-	}
-}
-
 func ls(cmd *cobra.Command, args []string) (err error) {
 
 	path := ""
@@ -131,11 +114,14 @@ func ls(cmd *cobra.Command, args []string) (err error) {
 	}
 
 	arg := files.NewListFolderArg(path)
-	arg.Recursive, _ = cmd.Flags().GetBool("recurse")
+	arg.Recursive = lsRecursive(cmd)
 	arg.IncludeDeleted, _ = cmd.Flags().GetBool("include-deleted")
 	onlyDeleted, _ := cmd.Flags().GetBool("only-deleted")
 	arg.IncludeDeleted = arg.IncludeDeleted || onlyDeleted
-	opts := parseLsOptions(cmd)
+	opts, err := parseListOptions(cmd)
+	if err != nil {
+		return err
+	}
 	if opts.limit > 0 {
 		if opts.limit > maxListFolderLimit {
 			return invalidArgumentsErrorWithDetails("`ls --limit` is too large", flagErrorDetails("limit"))
@@ -211,6 +197,12 @@ func metadataLimitReached(entries []files.IsMetadata, limit uint64) bool {
 	return limit > 0 && uint64(len(entries)) >= limit
 }
 
+func lsRecursive(cmd *cobra.Command) bool {
+	recursive, _ := cmd.Flags().GetBool("recursive")
+	recurse, _ := cmd.Flags().GetBool("recurse")
+	return recursive || recurse
+}
+
 func prepareLsEntries(dbx files.Client, entries []files.IsMetadata, onlyDeleted bool) ([]files.IsMetadata, error) {
 	var filtered []files.IsMetadata
 	for _, entry := range entries {
@@ -383,7 +375,8 @@ func init() {
 	RootCmd.AddCommand(lsCmd)
 
 	lsCmd.Flags().BoolP("long", "l", false, "Long listing")
-	lsCmd.Flags().BoolP("recurse", "R", false, "Recursively list all subfolders")
+	lsCmd.Flags().Bool("recursive", false, "Recursively list all subfolders")
+	lsCmd.Flags().BoolP("recurse", "R", false, "Alias for --recursive")
 	lsCmd.Flags().BoolP("include-deleted", "d", false, "Include deleted files")
 	lsCmd.Flags().BoolP("only-deleted", "D", false, "Only show deleted files")
 	lsCmd.Flags().Uint64("limit", 0, "Maximum number of entries to return")

cmd/ls_test.go

@@ -181,7 +181,7 @@ func TestRenderLsResultsShortModeUsesFourColumns(t *testing.T) {
 func TestLsJSONListsResultsAndInput(t *testing.T) {
 	cmd, stdout := testLsCmd(t)
 	setLsOutputJSON(t, cmd)
-	setLsFlag(t, cmd, "recurse", "true")
+	setLsFlag(t, cmd, "recursive", "true")
 	setLsFlag(t, cmd, "include-deleted", "true")
 	setLsFlag(t, cmd, "limit", "50")
 	setLsFlag(t, cmd, "long", "true")
@@ -315,6 +315,35 @@ func TestLsLimitStopsPaginationAndTruncatesOutput(t *testing.T) {
 	}
 }
 
+func TestLsRecurseAliasSetsRecursiveListFolderArg(t *testing.T) {
+	cmd, stdout := testLsCmd(t)
+	setLsOutputJSON(t, cmd)
+	setLsFlag(t, cmd, "recurse", "true")
+
+	var listArg *files.ListFolderArg
+	mock := &mockFilesClient{
+		listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) {
+			listArg = arg
+			return &files.ListFolderResult{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.Recursive {
+		t.Fatal("ListFolder recursive = false, want true from --recurse alias")
+	}
+	got := decodeLsOutput(t, stdout)
+	if !got.Input.Recursive {
+		t.Fatal("input recursive = false, want true from --recurse alias")
+	}
+}
+
 func TestLsLimitRejectsUint32Overflow(t *testing.T) {
 	cmd, stdout := testLsCmd(t)
 	setLsFlag(t, cmd, "limit", "4294967296")
@@ -327,6 +356,42 @@ func TestLsLimitRejectsUint32Overflow(t *testing.T) {
 	}
 }
 
+func TestLsRejectsInvalidListOptions(t *testing.T) {
+	tests := []struct {
+		name  string
+		flag  string
+		value string
+	}{
+		{name: "sort", flag: "sort", value: "date"},
+		{name: "time", flag: "time", value: "created"},
+		{name: "time-format", flag: "time-format", value: "unix"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cmd, stdout := testLsCmd(t)
+			setLsFlag(t, cmd, tt.flag, tt.value)
+			stubFilesClient(t, &mockFilesClient{
+				listFolderFn: func(arg *files.ListFolderArg) (*files.ListFolderResult, error) {
+					t.Fatalf("ListFolder called for invalid --%s", tt.flag)
+					return nil, nil
+				},
+			})
+
+			err := ls(cmd, nil)
+			if err == nil {
+				t.Fatalf("expected invalid --%s error", tt.flag)
+			}
+			if !strings.Contains(err.Error(), tt.flag) || !strings.Contains(err.Error(), tt.value) {
+				t.Fatalf("error = %v, want flag and value", err)
+			}
+			if got := stdout.String(); got != "" {
+				t.Fatalf("stdout = %q, want empty output on validation error", got)
+			}
+		})
+	}
+}
+
 func TestLsJSONFilePathUsesMetadata(t *testing.T) {
 	cmd, stdout := testLsCmd(t)
 	setLsOutputJSON(t, cmd)
@@ -533,6 +598,7 @@ func testLsCmd(t *testing.T) (*cobra.Command, *bytes.Buffer) {
 	cmd := &cobra.Command{Use: "ls"}
 	cmd.SetOut(&stdout)
 	cmd.Flags().BoolP("long", "l", false, "")
+	cmd.Flags().Bool("recursive", false, "")
 	cmd.Flags().BoolP("recurse", "R", false, "")
 	cmd.Flags().BoolP("include-deleted", "d", false, "")
 	cmd.Flags().BoolP("only-deleted", "D", false, "")

cmd/revs.go

@@ -50,14 +50,17 @@ func revs(cmd *cobra.Command, args []string) (err error) {
 		arg.Limit = limit
 	}
 
+	opts, err := parseListOptions(cmd)
+	if err != nil {
+		return err
+	}
+
 	dbx := filesNewFunc(config)
 	res, err := dbx.ListRevisions(arg)
 	if err != nil {
 		return
 	}
 
-	opts := parseLsOptions(cmd)
-
 	return renderRevisionsOutput(cmd, path, limit, res.Entries, opts)
 }
 

cmd/revs_test.go

@@ -180,6 +180,41 @@ func TestRevsJSONErrorWritesNoOutput(t *testing.T) {
 	}
 }
 
+func TestRevsRejectsInvalidListOptions(t *testing.T) {
+	tests := []struct {
+		name  string
+		flag  string
+		value string
+	}{
+		{name: "time", flag: "time", value: "created"},
+		{name: "time-format", flag: "time-format", value: "unix"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cmd, stdout := testRevsCmd()
+			setRevsFlag(t, cmd, tt.flag, tt.value)
+			stubFilesClient(t, &mockFilesClient{
+				listRevisionsFn: func(arg *files.ListRevisionsArg) (*files.ListRevisionsResult, error) {
+					t.Fatalf("ListRevisions called for invalid --%s", tt.flag)
+					return nil, nil
+				},
+			})
+
+			err := revs(cmd, []string{"/report.pdf"})
+			if err == nil {
+				t.Fatalf("expected invalid --%s error", tt.flag)
+			}
+			if !strings.Contains(err.Error(), tt.flag) || !strings.Contains(err.Error(), tt.value) {
+				t.Fatalf("error = %v, want flag and value", err)
+			}
+			if got := stdout.String(); got != "" {
+				t.Fatalf("stdout = %q, want empty output on validation error", got)
+			}
+		})
+	}
+}
+
 func TestRevsCommandSupportsStructuredOutput(t *testing.T) {
 	if !commandSupportsStructuredOutput(revsCmd) {
 		t.Fatal("revs command should support structured output")

cmd/search.go

@@ -97,12 +97,16 @@ func parseSearchOptions(cmd *cobra.Command) (searchCommandOptions, error) {
 	content, _ := cmd.Flags().GetBool("content")
 	limit, _ := cmd.Flags().GetUint64("limit")
 	orderBy, _ := cmd.Flags().GetString("order-by")
+	listOpts, err := parseListOptions(cmd)
+	if err != nil {
+		return searchCommandOptions{}, err
+	}
 	if !validSearchOrderBy(orderBy) {
 		return searchCommandOptions{}, invalidArgumentsErrorWithDetails("`search --order-by` must be one of: relevance, modified", flagErrorDetails("order-by"))
 	}
 
 	return searchCommandOptions{
-		list:    parseLsOptions(cmd),
+		list:    listOpts,
 		content: content,
 		limit:   limit,
 		orderBy: orderBy,

cmd/search_test.go

@@ -43,6 +43,42 @@ func TestSearchOrderByValidation(t *testing.T) {
 	}
 }
 
+func TestSearchRejectsInvalidListOptions(t *testing.T) {
+	tests := []struct {
+		name  string
+		flag  string
+		value string
+	}{
+		{name: "sort", flag: "sort", value: "date"},
+		{name: "time", flag: "time", value: "created"},
+		{name: "time-format", flag: "time-format", value: "unix"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cmd, stdout := testSearchCmd()
+			setSearchFlag(t, cmd, tt.flag, tt.value)
+			stubFilesClient(t, &mockFilesClient{
+				searchV2Fn: func(arg *files.SearchV2Arg) (*files.SearchV2Result, error) {
+					t.Fatalf("SearchV2 called for invalid --%s", tt.flag)
+					return nil, nil
+				},
+			})
+
+			err := search(cmd, []string{"query"})
+			if err == nil {
+				t.Fatalf("expected invalid --%s error", tt.flag)
+			}
+			if !strings.Contains(err.Error(), tt.flag) || !strings.Contains(err.Error(), tt.value) {
+				t.Fatalf("error = %v, want flag and value", err)
+			}
+			if got := stdout.String(); got != "" {
+				t.Fatalf("stdout = %q, want empty output on validation error", got)
+			}
+		})
+	}
+}
+
 func TestRenderSearchResultsSeparatesMatchesWithNewlines(t *testing.T) {
 	entries := []files.IsMetadata{
 		&files.FileMetadata{

docs/commands/dbxcli_ls.md

@@ -25,7 +25,8 @@ dbxcli ls [flags] [<path>]
       --limit uint           Maximum number of entries to return
   -l, --long                 Long listing
   -D, --only-deleted         Only show deleted files
-  -R, --recurse              Recursively list all subfolders
+  -R, --recurse              Alias for --recursive
+      --recursive            Recursively list all subfolders
   -r, --reverse              Reverse sort order
       --sort string          Sort by: name, size, time, type
       --time string          Time field: server, client (default "server")