-
Notifications
You must be signed in to change notification settings - Fork 0
Add Marshal/Unmarshal functions for direct struct conversion #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the WalkthroughAdds Marshal and Unmarshal functions to the toon library, enabling direct conversion between Go structs and TOON format using reflection and struct tags. Includes comprehensive test coverage, documentation updates, and example programs demonstrating round-trip serialization workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Marshal as Marshal()
participant toJsonValue as toJsonValue()
participant Encode as Encode()
participant Unmarshal as Unmarshal()
participant fromJsonValue as fromJsonValue()
participant Target as Target<br/>(Go Value)
Client->>Marshal: Marshal(struct,<br/>options)
Marshal->>toJsonValue: Convert struct to<br/>JsonValue tree
toJsonValue->>toJsonValue: Recursively handle<br/>fields & tags
toJsonValue-->>Marshal: JsonValue
Marshal->>Encode: Encode(JsonValue,<br/>options)
Encode-->>Marshal: []byte (TOON)
Marshal-->>Client: []byte (TOON)
Client->>Unmarshal: Unmarshal([]byte,<br/>target pointer)
Unmarshal->>Encode: Decode([]byte)
Encode-->>Unmarshal: JsonValue
Unmarshal->>fromJsonValue: Populate target from<br/>JsonValue
fromJsonValue->>fromJsonValue: Recursively map fields<br/>& validate tags
fromJsonValue->>Target: Set field values
Unmarshal-->>Client: error (nil on success)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Comment |
Co-authored-by: tnfssc <29162020+tnfssc@users.noreply.github.com>
Co-authored-by: tnfssc <29162020+tnfssc@users.noreply.github.com>
Co-authored-by: tnfssc <29162020+tnfssc@users.noreply.github.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/toon/marshal_test.go (1)
332-344: Prefer standard library function.The
containsandfindSubstringhelper functions reinventstrings.Containsfrom the standard library.Replace the custom implementation with the standard library:
-// Helper function to check if a string contains a substring -func contains(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(s) > len(substr) && findSubstring(s, substr)) -} - -func findSubstring(s, substr string) bool { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return true - } - } - return false -} +// Helper function to check if a string contains a substring +func contains(s, substr string) bool { + return strings.Contains(s, substr) +}Note:
strings.Containsis already imported in the test file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(2 hunks)pkg/toon/marshal.go(1 hunks)pkg/toon/marshal_test.go(1 hunks)tests/test_issue_demonstration.go(1 hunks)tests/test_marshal_example.go(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
63-63: Hard tabs
Column: 1
(MD010, no-hard-tabs)
64-64: Hard tabs
Column: 1
(MD010, no-hard-tabs)
66-66: Hard tabs
Column: 1
(MD010, no-hard-tabs)
70-70: Hard tabs
Column: 1
(MD010, no-hard-tabs)
71-71: Hard tabs
Column: 1
(MD010, no-hard-tabs)
72-72: Hard tabs
Column: 1
(MD010, no-hard-tabs)
76-76: Hard tabs
Column: 1
(MD010, no-hard-tabs)
77-77: Hard tabs
Column: 1
(MD010, no-hard-tabs)
78-78: Hard tabs
Column: 1
(MD010, no-hard-tabs)
79-79: Hard tabs
Column: 1
(MD010, no-hard-tabs)
80-80: Hard tabs
Column: 1
(MD010, no-hard-tabs)
81-81: Hard tabs
Column: 1
(MD010, no-hard-tabs)
83-83: Hard tabs
Column: 1
(MD010, no-hard-tabs)
84-84: Hard tabs
Column: 1
(MD010, no-hard-tabs)
85-85: Hard tabs
Column: 1
(MD010, no-hard-tabs)
86-86: Hard tabs
Column: 1
(MD010, no-hard-tabs)
87-87: Hard tabs
Column: 1
(MD010, no-hard-tabs)
89-89: Hard tabs
Column: 1
(MD010, no-hard-tabs)
90-90: Hard tabs
Column: 1
(MD010, no-hard-tabs)
91-91: Hard tabs
Column: 1
(MD010, no-hard-tabs)
92-92: Hard tabs
Column: 1
(MD010, no-hard-tabs)
93-93: Hard tabs
Column: 1
(MD010, no-hard-tabs)
94-94: Hard tabs
Column: 1
(MD010, no-hard-tabs)
95-95: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (10)
README.md (1)
14-14: LGTM! Clear feature description.The Marshal/Unmarshal feature is accurately described and appropriately compared to
encoding/json.tests/test_marshal_example.go (2)
10-27: LGTM! Well-structured demonstration types.The struct definitions effectively demonstrate all key features: standard toon tags, unexported fields, and explicit exclusion with
toon:"-".
29-103: LGTM! Comprehensive demonstration of functionality.The example effectively demonstrates:
- Round-trip struct marshaling/unmarshaling
- Map handling
- Error handling patterns
- Field verification
This serves as excellent documentation for users.
tests/test_issue_demonstration.go (1)
1-69: LGTM! Excellent problem/solution demonstration.This file effectively illustrates the issue from #1 and shows how Marshal/Unmarshal solves it. The contrast between the old map-based approach and the new struct-based approach clearly demonstrates the value of this feature.
pkg/toon/marshal_test.go (1)
8-30: LGTM! Well-designed test fixtures.The test structs provide excellent coverage of different scenarios: primitives, collections, nested structures, and field exclusion patterns.
pkg/toon/marshal.go (5)
9-38: LGTM! Clean public API design.The Marshal and Unmarshal functions follow the familiar
encoding/jsonpattern, with clear documentation and proper error propagation.
40-79: LGTM! Solid reflection-based conversion.The pointer dereferencing loop and type conversions are well-implemented. The approach of converting all numerics to float64 aligns with JSON semantics, which is appropriate for TOON.
119-156: LGTM! Proper struct field handling.The struct tag parsing correctly handles
toon:"-"for exclusion and extracts field names from tags. The comma-splitting logic (lines 141-144) prepares for potential future support of options likeomitempty, though that's not currently implemented.
212-229: LGTM! Flexible string conversion.The string field handling includes type coercion from numerics and booleans, which matches the PR's goal of supporting type conversions. This provides a user-friendly unmarshaling experience.
349-401: LGTM! Well-implemented struct population.The struct unmarshaling correctly:
- Maps TOON field names to struct fields using tags
- Ignores unknown fields (matching
encoding/jsonbehavior)- Provides helpful error messages with field context (line 394)
- Handles unexported fields and
toon:"-"exclusions
…`Unmarshal` as public API, and enhance testing with new examples and CI workflow.
The library lacked
json.Marshal/json.Unmarshalequivalents, forcing users to work withmap[string]interface{}and manual type assertions instead of idiomatic Go structs.Changes
New API functions (
pkg/toon/marshal.go):Marshal(v interface{}, options EncodeOptions) ([]byte, error)- converts Go values to TOON format using reflectionUnmarshal(data []byte, v interface{}, options DecodeOptions) error- parses TOON into Go valuesStruct tag support: Use
toon:"fieldname"tags likejsontags. Fields markedtoon:"-"or unexported are ignored.Type handling: Supports primitives, structs, maps, slices, arrays, and nested structures. Handles type conversions (e.g., numeric strings → string fields).
Usage
No changes to existing
Encode/Decodefunctions. 15 test cases cover struct marshaling, nested objects, arrays, maps, and type conversions.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.