Skip to content

Fixes2#54

Closed
moshloop wants to merge 4 commits intomainfrom
fixes2
Closed

Fixes2#54
moshloop wants to merge 4 commits intomainfrom
fixes2

Conversation

@moshloop
Copy link
Copy Markdown
Member

@moshloop moshloop commented Dec 14, 2025

Description

Brief description of the changes in this PR.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Performance improvement
  • Code refactoring

Testing

  • Tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested the CLI with example data

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

Breaking Changes

If this is a breaking change, please describe the impact and migration path for existing users:

Additional Notes

Add any additional notes, screenshots, or context about the changes here.

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced error diagnostics with improved command error logging
  • Improvements

    • Updated timestamp output format to RFC3339/ISO8601 standard (ISO8601 format with Z suffix) in JSON and YAML formatters
    • Refined tree and table formatting options handling for more flexible control
  • Tests

    • Disabled several test cases for revision
    • Updated test assertions for timestamp format changes

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 14, 2025

Walkthrough

The PR refactors flag and option handling for Tree and Table formatters by converting them from boolean to pointer-to-boolean types, introduces new reflection-based utility functions, adds CSS class deduplication logic, enhances error and debug logging, and disables several tests via function renaming.

Changes

Cohort / File(s) Summary
Flag and Option Refactoring
flags.go, formatters/options.go, formatters/manager.go
Converts Tree and Table flags from bool to *bool using BoolFunc registrations and updates option handling to validate nil pointers before dereferencing. Adds SkipTable() and SkipTree() convenience methods to FormatOptions.
Formatter Parser Updates
formatters/parser.go
Removes internal utility helpers (isEmptyValue, GetFieldValueCaseInsensitive, etc.) and introduces option-driven formatting. Updates ToPrettyData signature to accept variadic TypeOptions parameter with new mergeTypeOptions aggregator function.
Reflection Utilities
formatters/reflect.go
New file providing reflection-based utilities: FlattenSlice, ToSlice, processSliceElement, safeDerefPointer, isEmptyValue, and GetFieldValueCaseInsensitive for safe pointer handling and flexible type conversions.
Logging Enhancements
cobra_command.go, exec/exec.go
Adds error logging with skip report level in AddNamedCommand error path. Changes command start log level from Debug to Trace.
CSS Style Deduplication
api/text.go
Introduces uniqueStyles helper to compute deduplicated, space-separated CSS classes from style strings and new class inputs.
Test Updates
formatters/tests/formatters_test.go, formatters/tests/map_fields_test.go, formatters/tests/parser_test.go, formatters/tests/sorting_test.go, formatters/tests/tree_test.go
Updates test expectations for date formatting (RFC3339 format), changes Table option type from bool to *bool, disables TestSortRows and TestCustomRenderFunction tests via X prefix, and comments out ParseDataWithSchema test case.

Possibly related PRs

  • chore: fix tests #50: Modifies api/text.go and changes Text style/class merging and rendering logic, directly related to the uniqueStyles deduplication mechanism introduced here.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is entirely composed of an empty template with all required sections either unchecked or blank, containing no actual information about the changes. Fill in all required sections: provide a brief description of changes, select the type of change (likely 'Code refactoring' and/or 'Performance improvement'), confirm testing status, and complete the checklist items.
Docstring Coverage ⚠️ Warning Docstring coverage is 74.07% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Fixes2' is extremely vague and generic, providing no meaningful information about the primary changes in the pull request. Replace 'Fixes2' with a specific, descriptive title that summarizes the main change, such as 'Refactor Tree/Table formatting to use pointer-based boolean flags' or similar.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fixes2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
formatters/tests/map_fields_test.go (1)

55-86: Fix syntax error from incomplete comment block.

The test code starting at line 55 is commented out with //FIXME, but the test body (lines 62-86) remains uncommented, creating invalid syntax. The closing brace at line 86 doesn't match any opening brace, causing the compilation error flagged by the pipeline.

Apply this diff to properly comment out the entire disabled test:

 	// Test ParseDataWithSchema
 	//FIXME
-	// t.Run("ParseDataWithSchema", func(t *testing.T) {
-	// 	prettyData, err := parser.ParseDataWithSchema(testData, schema)
-	// 	if err != nil {
-	// 		t.Fatalf("ParseDataWithSchema failed: %v", err)
-	// 	}
-
-		// Check that scalar fields are parsed
-		if _, exists := prettyData.GetValue("name"); !exists {
-			t.Error("name field not found in Values")
-		}
-		if _, exists := prettyData.GetValue("age"); !exists {
-			t.Error("age field not found in Values")
-		}
-
-		// Check that map fields are parsed
-		if _, exists := prettyData.GetValue("address"); !exists {
-			t.Error("address map field not found in Values")
-		}
-		if _, exists := prettyData.GetValue("metadata"); !exists {
-			t.Error("metadata map field not found in Values")
-		}
-
-		// Check that table data is parsed
-		if _, exists := prettyData.GetTable("items"); !exists {
-			t.Error("items table not found in Tables")
-		}
-
-		if table, exists := prettyData.GetTable("items"); exists && len(table.Rows) != 2 {
-			t.Errorf("Expected 2 items in table, got %d", len(table.Rows))
-		}
-	})
+	// t.Run("ParseDataWithSchema", func(t *testing.T) {
+	// 	prettyData, err := parser.ParseDataWithSchema(testData, schema)
+	// 	if err != nil {
+	// 		t.Fatalf("ParseDataWithSchema failed: %v", err)
+	// 	}
+	//
+	// 	// Check that scalar fields are parsed
+	// 	if _, exists := prettyData.GetValue("name"); !exists {
+	// 		t.Error("name field not found in Values")
+	// 	}
+	// 	if _, exists := prettyData.GetValue("age"); !exists {
+	// 		t.Error("age field not found in Values")
+	// 	}
+	//
+	// 	// Check that map fields are parsed
+	// 	if _, exists := prettyData.GetValue("address"); !exists {
+	// 		t.Error("address map field not found in Values")
+	// 	}
+	// 	if _, exists := prettyData.GetValue("metadata"); !exists {
+	// 		t.Error("metadata map field not found in Values")
+	// 	}
+	//
+	// 	// Check that table data is parsed
+	// 	if _, exists := prettyData.GetTable("items"); !exists {
+	// 		t.Error("items table not found in Tables")
+	// 	}
+	//
+	// 	if table, exists := prettyData.GetTable("items"); exists && len(table.Rows) != 2 {
+	// 		t.Errorf("Expected 2 items in table, got %d", len(table.Rows))
+	// 	}
+	// })
formatters/options.go (1)

37-38: *Fix field types: Tree and Table must be bool, not bool.

Lines 37-38 declare Tree and Table as bool, but all the new code in this file (lines 48-54, 80-85, 145-153, 174-182) and related files (formatters/manager.go, flags.go) treats them as *bool with nil-checks and pointer dereferencing. This fundamental type mismatch is causing compilation failures throughout the codebase.

Apply this diff to fix the field declarations:

 	// Display structure flags (additive with format flags)
-	Tree  bool `json:"tree,omitempty"`  // Display in tree structure
-	Table bool `json:"table,omitempty"` // Display in table structure
+	Tree  *bool `json:"tree,omitempty"`  // Display in tree structure
+	Table *bool `json:"table,omitempty"` // Display in table structure

This change will resolve the compilation errors in:

  • formatters/manager.go (lines 211, 213)
  • formatters/options.go (lines 49, 53, 80-85, 146-147, 150-152, 175-176, 179-181)
  • flags.go (lines 70, 75)
🧹 Nitpick comments (2)
formatters/reflect.go (2)

80-140: Consider early return consistency and potential nil slice input.

Two observations:

  1. Lines 97-105: Excessive blank lines between result = append(...) and the else branches reduce readability.
  2. The function returns (nil, false) for empty input, but (result, len(result) > 0) at line 107 returns ([], false) when the input slice is non-empty but contains zero elements that match type T—this is inconsistent with Case 2 which returns (result, len(result) > 0) at line 139.
 				if typed, ok := elem.Interface().(T); ok {
 					result = append(result, typed)
-
 				} else {
-
 					return nil, false // Not all elements are T
 				}
 			} else {
-
 				return nil, false
 			}

207-214: Redundant case-folding: strings.EqualFold already handles case-insensitive comparison.

strings.EqualFold(field.Name, lowerName) compares field.Name against a pre-lowercased name. This works but is slightly misleading—EqualFold performs its own case-insensitive comparison, so pre-lowering name is unnecessary.

 	// Try case-insensitive match
-	lowerName := strings.ToLower(name)
 	for i := 0; i < typ.NumField(); i++ {
 		field := typ.Field(i)
-		if strings.EqualFold(field.Name, lowerName) {
+		if strings.EqualFold(field.Name, name) {
 			return val.Field(i)
 		}
 	}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dcc082d and 0cf673f.

📒 Files selected for processing (13)
  • api/text.go (1 hunks)
  • cobra_command.go (1 hunks)
  • exec/exec.go (1 hunks)
  • flags.go (1 hunks)
  • formatters/manager.go (1 hunks)
  • formatters/options.go (5 hunks)
  • formatters/parser.go (3 hunks)
  • formatters/reflect.go (1 hunks)
  • formatters/tests/formatters_test.go (3 hunks)
  • formatters/tests/map_fields_test.go (1 hunks)
  • formatters/tests/parser_test.go (1 hunks)
  • formatters/tests/sorting_test.go (1 hunks)
  • formatters/tests/tree_test.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
formatters/tests/parser_test.go (1)
formatters/options.go (1)
  • FormatOptions (18-46)
formatters/options.go (2)
format.go (1)
  • FormatOptions (17-17)
api/icons/icons.go (1)
  • Table (195-195)
cobra_command.go (1)
format.go (1)
  • Errorf (51-53)
api/text.go (2)
format.go (1)
  • Text (112-117)
api/tailwind/tailwind.go (1)
  • Style (244-261)
formatters/parser.go (1)
api/meta.go (1)
  • PrettyData (18-24)
exec/exec.go (3)
format.go (2)
  • Tracef (65-69)
  • Text (112-117)
api/text.go (1)
  • Text (50-57)
api/icons/icons.go (2)
  • MinimalArrow (156-156)
  • Add (211-211)
🪛 GitHub Actions: Lint
formatters/parser.go

[error] 558-558: golangci-lint: undefined: api.TypeOptions

formatters/tests/map_fields_test.go

[error] 86-86: Go compiler error: expected declaration, found ')'

🪛 GitHub Actions: PR #54
formatters/parser.go

[error] 558-558: Build failed: undefined: api.TypeOptions (formatters/parser.go:558:35) during 'go build -o clicky ./cmd/clicky/'

🪛 GitHub Actions: Test
formatters/parser.go

[error] 558-558: compile error: undefined: api.TypeOptions in formatters/parser.go:558

🪛 GitHub Check: golangci-lint
formatters/manager.go

[failure] 213-213:
invalid operation: cannot indirect options.Table (variable of type bool)


[failure] 213-213:
invalid operation: options.Table != nil (mismatched types bool and untyped nil)


[failure] 211-211:
invalid operation: cannot indirect options.Tree (variable of type bool)


[failure] 211-211:
invalid operation: options.Tree != nil (mismatched types bool and untyped nil)

formatters/options.go

[failure] 49-49:
invalid operation: cannot indirect o.Tree (variable of type bool)


[failure] 49-49:
invalid operation: o.Tree != nil (mismatched types bool and untyped nil)


[failure] 49-49:
invalid operation: cannot indirect o.Table (variable of type bool)


[failure] 49-49:
invalid operation: o.Table != nil (mismatched types bool and untyped nil)

formatters/parser.go

[failure] 568-568:
undefined: api.TypeOptions


[failure] 558-558:
undefined: api.TypeOptions

🪛 GitHub Check: Lint
formatters/manager.go

[failure] 213-213:
invalid operation: cannot indirect options.Table (variable of type bool)


[failure] 213-213:
invalid operation: options.Table != nil (mismatched types bool and untyped nil)


[failure] 211-211:
invalid operation: cannot indirect options.Tree (variable of type bool)


[failure] 211-211:
invalid operation: options.Tree != nil (mismatched types bool and untyped nil)

formatters/options.go

[failure] 49-49:
invalid operation: cannot indirect o.Tree (variable of type bool)


[failure] 49-49:
invalid operation: o.Tree != nil (mismatched types bool and untyped nil)


[failure] 49-49:
invalid operation: cannot indirect o.Table (variable of type bool)


[failure] 49-49:
invalid operation: o.Table != nil (mismatched types bool and untyped nil)

formatters/parser.go

[failure] 568-568:
undefined: api.TypeOptions


[failure] 558-558:
undefined: api.TypeOptions

🪛 GitHub Check: Test (ubuntu-latest, Go 1.25)
formatters/manager.go

[failure] 213-213:
invalid operation: cannot indirect options.Table (variable of type bool)


[failure] 213-213:
invalid operation: options.Table != nil (mismatched types bool and untyped nil)


[failure] 211-211:
invalid operation: cannot indirect options.Tree (variable of type bool)


[failure] 211-211:
invalid operation: options.Tree != nil (mismatched types bool and untyped nil)

formatters/options.go

[failure] 49-49:
invalid operation: cannot indirect o.Tree (variable of type bool)


[failure] 49-49:
invalid operation: o.Tree != nil (mismatched types bool and untyped nil)


[failure] 49-49:
invalid operation: cannot indirect o.Table (variable of type bool)


[failure] 49-49:
invalid operation: o.Table != nil (mismatched types bool and untyped nil)

formatters/parser.go

[failure] 568-568:
undefined: api.TypeOptions


[failure] 558-558:
undefined: api.TypeOptions

🔇 Additional comments (6)
exec/exec.go (1)

586-586: LGTM: Appropriate log level adjustment.

Changing from Debugf to Tracef correctly increases verbosity for detailed command execution tracing, consistent with the trace-level output at line 636.

cobra_command.go (1)

175-175: LGTM: Enhanced error logging.

Adding error logging before returning provides better debugging visibility for command failures. The use of WithSkipReportLevel(1) correctly adjusts the call stack for accurate error reporting.

formatters/reflect.go (2)

11-73: LGTM! Well-structured slice flattening with proper nil handling.

The function correctly handles edge cases: non-slice inputs, empty slices, nil outer elements, and interface dereferencing. The logic flow is clear and the comments are helpful.


142-166: > Likely an incorrect or invalid review comment.

formatters/tests/formatters_test.go (2)

88-90: LGTM! Test expectation aligned with RFC3339 format.

The updated expectation "2024-01-15T10:30:00Z" correctly reflects ISO8601/RFC3339 formatting for JSON output, which is the standard for interoperable date serialization.


123-125: LGTM! Consistent date format expectation for YAML.

The YAML formatter now uses the same RFC3339 format as JSON, ensuring consistency across structured output formats.

Comment thread api/text.go
Comment on lines +217 to +241
func uniqueStyles(existing string, styles ...string) string {
styleSet := make(map[string]struct{})
if existing != "" {
for _, s := range strings.Split(existing, " ") {
styleSet[s] = struct{}{}
}
}
for _, style := range styles {
for _, s := range strings.Split(style, " ") {
if s != "" {
styleSet[s] = struct{}{}
}
}
}
uniq := ""
for s := range styleSet {
if uniq == "" {
uniq = s
} else {
uniq += " " + s
}
}

return uniq
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Non-deterministic style ordering due to map iteration.

The uniqueStyles function builds the result by iterating over a map (lines 232-237), which has non-deterministic order in Go. This will produce inconsistent CSS class ordering across runs.

Apply this diff to ensure consistent, sorted output:

 func uniqueStyles(existing string, styles ...string) string {
 	styleSet := make(map[string]struct{})
 	if existing != "" {
 		for _, s := range strings.Split(existing, " ") {
 			styleSet[s] = struct{}{}
 		}
 	}
 	for _, style := range styles {
 		for _, s := range strings.Split(style, " ") {
 			if s != "" {
 				styleSet[s] = struct{}{}
 			}
 		}
 	}
-	uniq := ""
+	// Convert to sorted slice for deterministic output
+	uniqueList := make([]string, 0, len(styleSet))
 	for s := range styleSet {
-		if uniq == "" {
-			uniq = s
-		} else {
-			uniq += " " + s
-		}
+		uniqueList = append(uniqueList, s)
 	}
-
-	return uniq
+	sort.Strings(uniqueList)
+	return strings.Join(uniqueList, " ")
 }

Note: You'll need to add "sort" to the imports at the top of the file.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In api/text.go around lines 217 to 241, iteration over the map `styleSet`
produces non-deterministic class ordering; change the function to collect map
keys into a slice, sort the slice (add "sort" to imports), then join the sorted
keys with spaces to build and return a deterministic string of unique styles.

Comment thread flags.go
Comment on lines +69 to +77
flags.BoolFunc("tree", "Display in tree structure (additive with format)", func(s string) error {
Flags.Tree = lo.ToPtr(s == "true")
return nil
})
flags.BoolFunc("table", "Display in table structure (additive with format), or false to disable tables",
func(b string) error {
Flags.Table = lo.ToPtr(b == "true")
return nil
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

*Fix type mismatch: assigning bool to bool field.

Lines 70 and 75 assign lo.ToPtr(...) (which returns *bool) to Flags.Tree and Flags.Table, but these fields are declared as bool in the FormatOptions struct, not *bool. This causes the same compilation errors seen in formatters/manager.go.

The field declarations must be updated in formatters/options.go:

// In formatters/options.go FormatOptions struct (lines 37-38):
Tree  *bool `json:"tree,omitempty"`  // Display in tree structure
Table *bool `json:"table,omitempty"` // Display in table structure

Once corrected, this pointer-based flag binding will work as intended.

🤖 Prompt for AI Agents
In flags.go around lines 69 to 77 there is a type mismatch: the flag handlers
assign lo.ToPtr(...) (type *bool) to Flags.Tree and Flags.Table, but those
fields are declared as bool; update the FormatOptions fields in
formatters/options.go (where Tree and Table are defined, around lines 37-38) to
be pointer types (Tree *bool `json:"tree,omitempty"` and Table *bool
`json:"table,omitempty"`) so the flag binding uses the correct *bool types and
the compilation errors are resolved.

Comment thread formatters/manager.go
Comment on lines +211 to +213
if options.Tree != nil && *options.Tree {
return f.treeFormatter.Format(data...)
} else if options.Table {
} else if options.Table != nil && *options.Table {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

*Fix type mismatch: Tree and Table are bool, not bool.

The code attempts nil checks and pointer dereferencing on options.Tree and options.Table, but these fields are declared as bool in FormatOptions (line 37-38 in formatters/options.go), not *bool. This causes compilation failures across multiple files.

The field declarations in FormatOptions need to be updated to pointer types:

// In formatters/options.go, update lines 37-38:
Tree  *bool `json:"tree,omitempty"`  // Display in tree structure
Table *bool `json:"table,omitempty"` // Display in table structure

Once the field types are corrected, this pointer-checking logic will work correctly.

🧰 Tools
🪛 GitHub Check: golangci-lint

[failure] 213-213:
invalid operation: cannot indirect options.Table (variable of type bool)


[failure] 213-213:
invalid operation: options.Table != nil (mismatched types bool and untyped nil)


[failure] 211-211:
invalid operation: cannot indirect options.Tree (variable of type bool)


[failure] 211-211:
invalid operation: options.Tree != nil (mismatched types bool and untyped nil)

🪛 GitHub Check: Lint

[failure] 213-213:
invalid operation: cannot indirect options.Table (variable of type bool)


[failure] 213-213:
invalid operation: options.Table != nil (mismatched types bool and untyped nil)


[failure] 211-211:
invalid operation: cannot indirect options.Tree (variable of type bool)


[failure] 211-211:
invalid operation: options.Tree != nil (mismatched types bool and untyped nil)

🪛 GitHub Check: Test (ubuntu-latest, Go 1.25)

[failure] 213-213:
invalid operation: cannot indirect options.Table (variable of type bool)


[failure] 213-213:
invalid operation: options.Table != nil (mismatched types bool and untyped nil)


[failure] 211-211:
invalid operation: cannot indirect options.Tree (variable of type bool)


[failure] 211-211:
invalid operation: options.Tree != nil (mismatched types bool and untyped nil)

🤖 Prompt for AI Agents
In formatters/manager.go around lines 211-213 the code treats options.Tree and
options.Table as *bool, but FormatOptions currently declares them as bool;
update the declarations in formatters/options.go (around lines 37-38) to be
pointer types (Tree *bool and Table *bool) so the nil checks and dereferences
are valid, then rebuild to catch and fix any remaining type mismatches in other
files.

Comment thread formatters/parser.go
Comment on lines +558 to +568
func mergeTypeOptions(opts ...api.TypeOptions) api.TypeOptions {
merged := api.TypeOptions{}
for _, opt := range opts {
merged.SkipTable = merged.SkipTable || opt.SkipTable
merged.SkipTree = merged.SkipTree || opt.SkipTree
}
return merged
}

// ToPrettyData converts various input types to PrettyData
func ToPrettyData(data interface{}) (*api.PrettyData, error) {
func ToPrettyData(data interface{}, opts ...api.TypeOptions) (*api.PrettyData, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix undefined type: api.TypeOptions does not exist.

The code references api.TypeOptions in the mergeTypeOptions function (line 558) and ToPrettyData signature (line 568), but this type is undefined, causing compilation failures across all pipelines.

The api.TypeOptions type needs to be defined in the api package. Based on usage context (lines 561-562 checking SkipTable and SkipTree), it should likely be:

// In api package (e.g., api/meta.go or api/options.go):
type TypeOptions struct {
	SkipTable bool
	SkipTree  bool
}

Alternatively, if the intent was to use FormatOptions from the formatters package, the signature should be:

func ToPrettyData(data interface{}, opts ...FormatOptions) (*api.PrettyData, error) {
	// And update mergeTypeOptions to use FormatOptions
}

Do you want me to generate the complete fix based on the intended design?

🧰 Tools
🪛 GitHub Actions: Lint

[error] 558-558: golangci-lint: undefined: api.TypeOptions

🪛 GitHub Actions: PR #54

[error] 558-558: Build failed: undefined: api.TypeOptions (formatters/parser.go:558:35) during 'go build -o clicky ./cmd/clicky/'

🪛 GitHub Actions: Test

[error] 558-558: compile error: undefined: api.TypeOptions in formatters/parser.go:558

🪛 GitHub Check: golangci-lint

[failure] 568-568:
undefined: api.TypeOptions


[failure] 558-558:
undefined: api.TypeOptions

🪛 GitHub Check: Lint

[failure] 568-568:
undefined: api.TypeOptions


[failure] 558-558:
undefined: api.TypeOptions

🪛 GitHub Check: Test (ubuntu-latest, Go 1.25)

[failure] 568-568:
undefined: api.TypeOptions


[failure] 558-558:
undefined: api.TypeOptions

🤖 Prompt for AI Agents
In formatters/parser.go around lines 558 to 568, the code references an
undefined type api.TypeOptions in mergeTypeOptions and ToPrettyData; define the
type in the api package or switch usages to the correct existing type. Either
add a new type in the api package (e.g., TypeOptions with SkipTable and SkipTree
bool fields) and update imports, or change mergeTypeOptions and ToPrettyData to
accept the existing formatters.FormatOptions (or api.FormatOptions) type used
elsewhere; ensure all call sites compile by updating signatures and any
construction of option values accordingly.

}

opts := FormatOptions{Table: true}
opts := FormatOptions{Table: lo.ToPtr(true)}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

rg -n 'samber/lo' formatters/tests/parser_test.go

Repository: flanksource/clicky

Length of output: 44


🏁 Script executed:

head -n 300 formatters/tests/parser_test.go | tail -n +1 | head -n 30

Repository: flanksource/clicky

Length of output: 617


🏁 Script executed:

sed -n '3,8p; 285,295p' formatters/tests/parser_test.go

Repository: flanksource/clicky

Length of output: 426


🏁 Script executed:

#!/bin/bash
# Check if lo is imported in this file
rg -n 'samber/lo' formatters/tests/parser_test.go

Repository: flanksource/clicky

Length of output: 44


Add missing import for lo package.

The lo.ToPtr call at line 290 requires the import github.com/samber/lo, which is not present in the imports section (lines 3-8).

🤖 Prompt for AI Agents
In formatters/tests/parser_test.go around line 290, the test uses lo.ToPtr but
the file is missing the import for github.com/samber/lo; add that package to the
import block (e.g. import "github.com/samber/lo"), save and run goimports/gofmt
to ensure imports are ordered and formatted correctly.

Comment on lines +290 to 291
opts := FormatOptions{Table: lo.ToPtr(true)}
prettyData, err := ToPrettyDataWithOptions(input, opts)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the FormatOptions struct definition
ast-grep --pattern 'type FormatOptions struct {
  $$$
}'

Repository: flanksource/clicky

Length of output: 180


🏁 Script executed:

#!/bin/bash
# Also search with rg to find FormatOptions definition across the codebase
rg 'type FormatOptions' -A 10

Repository: flanksource/clicky

Length of output: 1315


🏁 Script executed:

#!/bin/bash
# Read the formatters/options.go file to see the full struct
cat -n formatters/options.go

Repository: flanksource/clicky

Length of output: 9497


The struct definition at formatters/options.go line 38 should declare Table *bool, not Table bool.

The FormatOptions struct declares Table bool, but the codebase consistently treats it as a pointer (*bool): methods check != nil and dereference with *, and flag binding assigns lo.ToPtr() results to it. The test code at lines 290-291 correctly uses lo.ToPtr(true), but only works because the struct field definition itself is incorrect. Update line 38 to declare Table *bool to match the actual usage pattern.

🤖 Prompt for AI Agents
In formatters/options.go around line 38, the FormatOptions struct currently
declares Table as a non-pointer (Table bool) but the codebase expects a pointer;
change the field declaration to Table *bool so callers that check for nil and
use lo.ToPtr() work correctly, update any nearby comments if needed, run `go
test ./...` to verify and fix any compile errors that surface from the type
change.

)

func TestSortRows(t *testing.T) {
func XTestSortRows(t *testing.T) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for TODO/FIXME comments or issues referencing this disabled test
rg -n -i 'XTestSortRows|sortrows.*skip|sortrows.*disable' --type=go

Repository: flanksource/clicky

Length of output: 132


🏁 Script executed:

# Also check the test file itself for any comments around the disabled test
cat -n formatters/tests/sorting_test.go | head -20

Repository: flanksource/clicky

Length of output: 1402


🏁 Script executed:

# Search for any issue references or TODO comments in the file
rg -n 'TODO|FIXME|XXX' formatters/tests/sorting_test.go

Repository: flanksource/clicky

Length of output: 44


Test disabled via XTest prefix—add tracking or documentation for this disabled test.

TestSortRows has been renamed to XTestSortRows, which disables it from go test discovery. There is no TODO, FIXME, or issue reference explaining why this test is disabled. Either add an inline comment explaining the reason (e.g., "disabled pending refactor of SortRows") or create a tracking issue to ensure it is not forgotten.

🤖 Prompt for AI Agents
In formatters/tests/sorting_test.go around line 11, the test has been renamed
from TestSortRows to XTestSortRows which disables it from go test discovery; add
a brief inline comment on the same line or immediately above explaining why the
test is disabled (e.g., "disabled pending refactor of SortRows" or reference to
an issue/PR number), or create a tracking issue and reference its ID in the
comment; ensure the comment includes owner/context and a TODO/FIXME tag so it’s
discoverable by linters and future maintainers.

}

func TestCustomRenderFunction(t *testing.T) {
func XTestCustomRenderFunction(t *testing.T) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Second test disabled via XTest prefix—pattern raises concern.

Combined with XTestSortRows in sorting_test.go, this PR disables two tests without explanation. If these tests are failing due to the refactoring, consider:

  1. Adding t.Skip("reason") inside the test with an explanation (like line 156 does)
  2. Creating a tracking issue for re-enabling them
  3. Documenting in the PR description why these are temporarily disabled
🤖 Prompt for AI Agents
In formatters/tests/tree_test.go around line 128 the test was disabled by
renaming it to XTestCustomRenderFunction; replace the XTest prefix by restoring
the original Test name or, if you intend to keep it skipped temporarily, revert
the name to TestCustomRenderFunction and add an explicit t.Skip("brief reason
and tracking issue URL") as the first statement, create a tracking issue to
re-enable the test, and update the PR description with the reason and issue link
so disabled tests are documented and traceable.

@moshloop moshloop closed this Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant