fix: CodeRabbit auto-fixes for PR #102

cobra_command.go

@@ -1,12 +1,14 @@
 package clicky
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"reflect"
 	"strings"
 	"sync"
 
+	"github.com/flanksource/clicky/api"
 	"github.com/flanksource/clicky/flags"
 	"github.com/flanksource/commons/logger"
 	"github.com/samber/lo"
@@ -243,7 +245,19 @@ func AddNamedCommand[T any, R any](name string, parent *cobra.Command, opts T, f
 		// Call the function
 		result, err := fn(optsValue.Interface().(T))
 		if err != nil {
-			logger.GetSlogLogger().WithSkipReportLevel(2).Errorf("Command %s failed: %w", name, err)
+			// An error that carries a clicky rendering interface
+			// (Pretty/Textable/Tree*) is rendered through the same format
+			// pipeline as a success result — honouring --format — instead of
+			// being collapsed to its Error() line. The error is still
+			// returned so cobra exits non-zero.
+			if rich, ok := renderableError(err); ok {
+				if specErr := Flags.ParseFormatSpec(); specErr != nil {
+					return specErr
+				}
+				PrintAndWriteSinks(rich, Flags.FormatOptions)
+			} else {
+				logger.GetSlogLogger().WithSkipReportLevel(2).Errorf("Command %s failed: %v", name, err)
+			}
 			return err
 		}
 
@@ -267,6 +281,32 @@ func isStdinAvailable() bool {
 	return (stat.Mode() & os.ModeCharDevice) == 0
 }
 
+// renderableError reports whether err — or any error in its Unwrap chain —
+// carries a clicky rendering interface (Pretty / Textable / Tree*), so the
+// command runner can render it through the format pipeline instead of just
+// logging Error(). The chain is walked so a fmt-wrapped rich error still
+// renders. The returned value is the matched error, ready to hand to
+// PrintAndWriteSinks.
+func renderableError(err error) (any, bool) {
+	for e := err; e != nil; {
+		if api.TryTypedValue(e) != nil {
+			return e, true
+		}
+		// Check if this error wraps multiple errors (joined errors)
+		if unwrapper, ok := e.(interface{ Unwrap() []error }); ok {
+			for _, child := range unwrapper.Unwrap() {
+				if result, found := renderableError(child); found {
+					return result, true
+				}
+			}
+			return nil, false
+		}
+		// Continue with single-error chain
+		e = errors.Unwrap(e)
+	}
+	return nil, false
+}
+
 type Name interface {
 	GetName() string
 }

cobra_command_test.go

@@ -0,0 +1,48 @@
+package clicky
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+
+	"github.com/flanksource/clicky/api"
+)
+
+// prettyErr is a test error that carries a clicky rendering interface —
+// the shape renderableError must detect so the command runner renders it
+// through the format pipeline instead of collapsing it to Error().
+type prettyErr struct{ msg string }
+
+func (e *prettyErr) Error() string    { return e.msg }
+func (e *prettyErr) Pretty() api.Text { return api.Text{Content: "rendered: " + e.msg} }
+
+func TestRenderableError_DetectsErrorImplementingPretty(t *testing.T) {
+	rich, ok := renderableError(&prettyErr{msg: "boom"})
+	if !ok {
+		t.Fatalf("renderableError must detect an error implementing Pretty()")
+	}
+	if api.TryTypedValue(rich) == nil {
+		t.Fatalf("the returned value must be renderable by the format pipeline")
+	}
+}
+
+func TestRenderableError_WalksWrappedChain(t *testing.T) {
+	// A fmt-wrapped rich error must still be detected — clicky walks the
+	// Unwrap chain so a renderable cause survives an outer fmt.Errorf.
+	wrapped := fmt.Errorf("dispatching: %w", &prettyErr{msg: "boom"})
+	rich, ok := renderableError(wrapped)
+	if !ok {
+		t.Fatalf("renderableError must walk the Unwrap chain to find a renderable cause")
+	}
+	if _, isPretty := rich.(*prettyErr); !isPretty {
+		t.Fatalf("renderableError must return the matched *prettyErr, got %T", rich)
+	}
+}
+
+func TestRenderableError_PlainErrorIsNotRenderable(t *testing.T) {
+	// A plain error carries no rendering interface — the command runner
+	// must fall back to logging Error(), not the render pipeline.
+	if _, ok := renderableError(errors.New("plain failure")); ok {
+		t.Fatalf("a plain error must not be reported as renderable")
+	}
+}

entity.go

@@ -93,6 +93,11 @@ type ActionInfo struct {
 	// and populated into the flag map passed to DataFunc.
 	FlagsType    reflect.Type
 	ResponseType reflect.Type
+	// OptionalID, when true, makes the positional <id> argument optional on
+	// the generated action command — the action is invokable with no
+	// argument (and the `id` passed to the run func is empty). Use for
+	// actions whose target is supplied entirely through flags.
+	OptionalID bool
 }
 
 // BulkActionInfo is the type-erased representation of a bulk action.
@@ -135,11 +140,12 @@ type EntityAction interface {
 }
 
 type actionSpec[R any] struct {
-	name   string
-	short  string
-	method string
-	run    func(id string, flags map[string]string) (R, error)
-	flags  ActionFlags
+	name       string
+	short      string
+	method     string
+	run        func(id string, flags map[string]string) (R, error)
+	flags      ActionFlags
+	optionalID bool
 }
 
 // Action creates a typed custom operation on a single entity by ID.
@@ -169,19 +175,28 @@ func (a *actionSpec[R]) WithMethod(method string) *actionSpec[R] {
 	return a
 }
 
+// WithOptionalID makes the positional <id> argument optional on the
+// generated action command. Use for actions whose target is supplied
+// entirely through flags; the `id` passed to the run func is then empty.
+func (a *actionSpec[R]) WithOptionalID() *actionSpec[R] {
+	a.optionalID = true
+	return a
+}
+
 func (a *actionSpec[R]) actionInfo() ActionInfo {
 	return ActionInfo{
 		Name:         a.name,
 		Short:        a.short,
 		Method:       a.method,
 		FlagsType:    actionFlagsType(a.flags),
 		ResponseType: responseTypeOf[R](),
+		OptionalID:   a.optionalID,
 		DataFunc: func(flagMap map[string]string, args []string) (any, error) {
 			id := flagMap["id"]
 			if id == "" && len(args) > 0 {
 				id = args[0]
 			}
-			if id == "" {
+			if id == "" && !a.optionalID {
 				return nil, fmt.Errorf("id is required")
 			}
 			return a.run(id, flagMap)
@@ -621,7 +636,7 @@ func generateEntityCLI(parent *cobra.Command, entity EntityInfo) {
 			DataFunc:     action.DataFunc,
 			FlagsType:    action.FlagsType,
 			ResponseType: action.ResponseType,
-		}, entity.ValidArgs, "action", "", "entity", action.Name, "id", false, false)
+		}, entity.ValidArgs, "action", "", "entity", action.Name, "id", false, false, action.OptionalID)
 	}
 
 	for _, ba := range entity.BulkActions {
@@ -647,6 +662,7 @@ func generateEntitySubcommand(parent *cobra.Command, entity EntityInfo, op Entit
 			"id",
 			false,
 			false,
+			false,
 		)
 	case "create":
 		generateBodyCommand(parent, "create", fmt.Sprintf("Create a %s", entity.Name), op)
@@ -666,6 +682,7 @@ func generateEntitySubcommand(parent *cobra.Command, entity EntityInfo, op Entit
 			"id",
 			false,
 			false,
+			false,
 		)
 	}
 }
@@ -720,16 +737,23 @@ func generateIDCommand(
 	idParam string,
 	supportsLookup bool,
 	supportsFilterMode bool,
+	optionalID bool,
 ) {
 	hasFlags := op.FlagsType != nil
-	use := fmt.Sprintf("%s <id>", verb)
+	idToken := "<id>"
+	args := cobra.ExactArgs(1)
+	if optionalID {
+		idToken = "[id]"
+		args = cobra.MaximumNArgs(1)
+	}
+	use := fmt.Sprintf("%s %s", verb, idToken)
 	if hasFlags {
-		use = fmt.Sprintf("%s <id> [flags]", verb)
+		use = fmt.Sprintf("%s %s [flags]", verb, idToken)
 	}
 	cmd := &cobra.Command{
 		Use:   use,
 		Short: short,
-		Args:  cobra.ExactArgs(1),
+		Args:  args,
 		RunE: func(c *cobra.Command, args []string) error {
 			var flagMap map[string]string
 			if hasFlags {

entity_filters_test.go

@@ -624,3 +624,76 @@ func TestEntityValidArgsPropagateToGeneratedIDCommands(t *testing.T) {
 		}
 	}
 }
+
+func TestActionWithOptionalIDAcceptsNoPositionalArg(t *testing.T) {
+	resetEntityRegistry(t)
+	defer resetEntityRegistry(t)
+
+	RegisterEntity(Entity[entityFilterTestEntity, entityFilterTestOpts, any]{
+		Name: "optional-id-entity",
+		Get:  func(id string) (any, error) { return id, nil },
+		Actions: []EntityAction{
+			Action("scan", func(id string, _ map[string]string) (any, error) {
+				return id, nil
+			}).WithShort("Scan with no id").WithOptionalID(),
+			Action("restart", func(id string, _ map[string]string) (any, error) {
+				return id, nil
+			}).WithShort("Restart one entity"),
+		},
+	})
+
+	root := &cobra.Command{Use: "root"}
+	GenerateCLI(root)
+
+	scan, _, err := root.Find([]string{"optional-id-entity", "scan"})
+	if err != nil || scan == nil {
+		t.Fatalf("expected to find scan command, got err=%v", err)
+	}
+	// The optional-id action accepts zero args but still rejects more than one.
+	if err := scan.Args(scan, nil); err != nil {
+		t.Fatalf("optional-id action must accept zero args, got: %v", err)
+	}
+	if err := scan.Args(scan, []string{"x", "y"}); err == nil {
+		t.Fatalf("optional-id action must still reject more than one arg")
+	}
+	if scan.Use != "scan [id]" {
+		t.Fatalf("optional-id action use should show [id], got %q", scan.Use)
+	}
+
+	// A normal action still forces exactly one positional arg.
+	restart, _, err := root.Find([]string{"optional-id-entity", "restart"})
+	if err != nil || restart == nil {
+		t.Fatalf("expected to find restart command, got err=%v", err)
+	}
+	if err := restart.Args(restart, nil); err == nil {
+		t.Fatalf("a normal action must still require its id arg")
+	}
+	if restart.Use != "restart <id>" {
+		t.Fatalf("normal action use should show <id>, got %q", restart.Use)
+	}
+}
+
+func TestActionInfoOptionalIDSkipsIDRequiredCheck(t *testing.T) {
+	var gotID = "sentinel"
+	spec := Action("scan", func(id string, _ map[string]string) (any, error) {
+		gotID = id
+		return id, nil
+	}).WithOptionalID()
+
+	// The DataFunc must not error on a missing id when OptionalID is set;
+	// the run func receives an empty id.
+	if _, err := spec.actionInfo().DataFunc(map[string]string{}, nil); err != nil {
+		t.Fatalf("optional-id DataFunc must not require an id: %v", err)
+	}
+	if gotID != "" {
+		t.Fatalf("expected empty id passed to run func, got %q", gotID)
+	}
+
+	// Without WithOptionalID the missing-id check still fires.
+	plain := Action("restart", func(id string, _ map[string]string) (any, error) {
+		return id, nil
+	})
+	if _, err := plain.actionInfo().DataFunc(map[string]string{}, nil); err == nil {
+		t.Fatalf("a normal action must still reject a missing id")
+	}
+}

examples/go.mod

@@ -100,6 +100,7 @@ require (
 	github.com/patrickmn/go-cache v2.1.0+incompatible // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/playwright-community/playwright-go v0.5700.1 // indirect
+	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/power-devops/perfstat v0.0.0-20210106213030-5aafc221ea8c // indirect
 	github.com/prometheus/client_golang v1.23.2 // indirect
 	github.com/prometheus/client_model v0.6.2 // indirect

examples/uber_demo/main.go

@@ -1306,10 +1306,15 @@ func showStackTrace(opts StackTraceOptions) (any, error) {
 		headersOnlyOpts = append(headersOnlyOpts, clicky.WithMaxStackFrames(opts.Max))
 	}
 
+	nativeOpts := []api.StackTraceOption{}
+	if opts.Max > 0 {
+		nativeOpts = append(nativeOpts, clicky.WithMaxStackFrames(opts.Max))
+	}
+
 	return StackTraceShowcase{
 		WithSource:    clicky.StackTrace(javaSampleTrace, resolvedOpts...),
 		WithoutSource: clicky.StackTrace(javaNullPointerTrace, headersOnlyOpts...),
-		NativeFrames:  clicky.StackTrace(javaNullPointerTrace, headersOnlyOpts...),
+		NativeFrames:  clicky.StackTrace(javaNullPointerTrace, nativeOpts...),
 	}, nil
 }
 

formatters/html_formatter.go

@@ -199,7 +199,10 @@ func (f *HTMLFormatter) format(in interface{}, options FormatOptions) (string, e
 		return f.wrapHTMLBody(trace.HTML()), nil
 	}
 	if trace, ok := in.(*api.StackTrace); ok {
-		return f.wrapHTMLBody(trace.HTML()), nil
+		if trace != nil {
+			return f.wrapHTMLBody(trace.HTML()), nil
+		}
+		return "", nil
 	}
 
 	// Check if input implements Pretty interface