feat: add json colored formatter #25
Conversation
WalkthroughThe changes update the JSON formatting functionality to include an optional colorized output. The Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant F as Formatter.Json
participant Fmt as formatJson
participant H as Helper Functions
C->>F: Call Json(j, colorized)
F->>Fmt: Invokes formatJson(j, colorized)
Fmt->>H: Format JSON key via formatJsonKey(key, colorized)
alt Process each JSON value by type
Fmt->>H: Call formatJsonStringValue/NumberValue/BoolValue/NilValue based on type
end
H-->>Fmt: Return formatted value
Fmt-->>F: Return complete formatted JSON string
F-->>C: Return (colorized) JSON output
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code Graph Analysis (1)pkg/printer/formatter_test.go (2)
🔇 Additional comments (8)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
pkg/printer/formatter_test.go (1)
191-201: Consider renaming or merging duplicate "nested" tests.
The “nested” test case appears twice with the same content and non-colorized output. It could be beneficial to unify these two test cases under a single scenario or rename one to differentiate them.Also applies to: 209-218
pkg/printer/formatter.go (1)
179-179: Remove or replace debug statement.
Thefmt.Printf("%T\n", vv)call appears to be leftover debugging. Consider removing it or switching to a structured logger if needed.fmt.Printf("%T\n", vv) - sb.WriteString(formatValue(vv, level)) + sb.WriteString(formatValue(vv, level))
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/printer/formatter.go(3 hunks)pkg/printer/formatter_test.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
pkg/printer/formatter_test.go (2)
pkg/printer/colors.go (5)
Yellow(27-29)Cyan(39-41)Green(23-25)Magenta(35-37)Red(19-21)pkg/printer/formatter.go (1)
Formatter(14-16)
pkg/printer/formatter.go (1)
pkg/printer/colors.go (5)
Yellow(27-29)Green(23-25)Cyan(39-41)Magenta(35-37)Red(19-21)
🪛 GitHub Actions: .github/workflows/ci.yml
pkg/printer/formatter_test.go
[error] 279-279: Test_formatJson/colored_list failed: expected equal; got: {"list": [1, "string", false, null]}; want: {"list": [1, "string", false, null, {"one": 1, "two": 2}]}
🔇 Additional comments (4)
pkg/printer/formatter_test.go (3)
177-180: Good addition of fields to the test case struct.
These new fields (name,json,useColor,want) enhance the flexibility of the test suite by permitting distinct inputs and colorization settings for thorough JSON formatting tests.
202-207: Comprehensive coverage for empty maps.
Covering edge cases like empty maps is crucial. This test scenario helps confirm that the formatter handles empty JSON objects gracefully.
277-277: Correct usage of the formatter with color-enabled tests.
InstantiatingFormatter{colored: tt.useColor}is consistent with the updated design and ensures the test logic toggles color output properly.pkg/printer/formatter.go (1)
162-193: Handle unrecognized data types or confirm limited type usage.
If this code is intended to support additional Go types (e.g.,int64,uint, custom structs), it will silently produce"". Verify that only these specific types (string, number, bool, nil, map, slice) will be passed in or handle others gracefully.Would you like a script to check for potential calls passing unsupported types to
Formatter.Jsonacross your codebase?
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
pkg/printer/formatter_test.go (1)
191-218:⚠️ Potential issueDuplicate test case: remove redundant "nested" test case.
There are two identical test cases named "nested" with the same input and expected output. The second instance (lines 209-218) should be removed to avoid redundancy.
- { - name: "nested", - json: map[string]any{ - "user": map[string]any{ - "id": 1, - "name": "Bob", - }, - }, - useColor: false, - want: "{\n \"user\": {\n \"id\": 1,\n \"name\": \"Bob\"\n }\n}", - },
🧹 Nitpick comments (1)
pkg/printer/formatter.go (1)
161-192: Consider improving array formatting for better readability of complex arrays.The current implementation doesn't add newlines or proper indentation for array elements, which could make the output less readable for complex arrays with nested structures. Consider enhancing the array formatting to add newlines and indentation when elements are maps or arrays themselves.
case []any: sb := strings.Builder{} sb.WriteString("[") + + hasComplexElements := false + for _, vv := range v { + switch vv.(type) { + case map[string]any, []any: + hasComplexElements = true + break + } + } + + padding := strings.Repeat(" ", level+1) for i, vv := range v { + isComplex := false + switch vv.(type) { + case map[string]any, []any: + isComplex = true + } + + if hasComplexElements && isComplex { + sb.WriteString("\n") + sb.WriteString(padding) + } + sb.WriteString(formatValue(vv, level)) if i != len(v)-1 { - sb.WriteString(", ") + if hasComplexElements { + sb.WriteString(",\n") + sb.WriteString(padding) + } else { + sb.WriteString(", ") + } } } + + if hasComplexElements { + sb.WriteString("\n") + sb.WriteString(strings.Repeat(" ", level)) + } sb.WriteString("]")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/printer/formatter.go(3 hunks)pkg/printer/formatter_test.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
pkg/printer/formatter_test.go (2)
pkg/printer/colors.go (5)
Yellow(27-29)Cyan(39-41)Green(23-25)Magenta(35-37)Red(19-21)pkg/printer/formatter.go (1)
Formatter(14-16)
pkg/printer/formatter.go (1)
pkg/printer/colors.go (5)
Yellow(27-29)Green(23-25)Cyan(39-41)Magenta(35-37)Red(19-21)
🔇 Additional comments (2)
pkg/printer/formatter.go (2)
30-32: Function signature inconsistency with AI summary.The AI summary indicates that the method signature was updated to
func (f Formatter) Json(j map[string]any, colorized bool) string, but the implementation still uses the original signature and passes the struct'scoloredfield instead. This implementation is correct and matches how the function is called in tests, but the summary is inaccurate.
116-240: LGTM: Effective implementation of colorized JSON formatting.The implementation effectively handles different JSON value types and applies appropriate colorization based on the type. The recursive approach with type-specific helper functions is clean and maintainable.
Summary by CodeRabbit
New Features
Tests