Skip to content
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

Add support for RawMessage, similar to json.RawMessage #668

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cpuguy83
Copy link

This adds a type that users can use to refer to raw data in the yaml for deferred decoding.

Closes #425

@codecov-commenter
Copy link

codecov-commenter commented Feb 20, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 46.66667% with 8 lines in your changes missing coverage. Please review.

Project coverage is 77.56%. Comparing base (abc7083) to head (e81dd30).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #668      +/-   ##
==========================================
- Coverage   77.61%   77.56%   -0.06%     
==========================================
  Files          22       22              
  Lines        7953     7968      +15     
==========================================
+ Hits         6173     6180       +7     
- Misses       1362     1368       +6     
- Partials      418      420       +2     

@asmfreak
Copy link

I think you'd want to see this pr merged #642 . It implements an even more erganomic feature

This adds a type that users can use to refer to raw data in the yaml for
deferred decoding.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83
Copy link
Author

I think you'd want to see this pr merged #642 . It implements an even more erganomic feature

That is an elegant solution since it doesn't require re-parsing the data.

@goccy
Copy link
Owner

goccy commented Feb 22, 2025

@cpuguy83 Thank you for your contribution !!

goccy/go-yaml provides a way to decode into ast.Node to defer evaluation, but adding yaml.RawMessage to handle raw byte slices while avoiding parsing costs sounds like a good idea. Actually, I was just about to implement this feature myself.
This also serves as an intuitive solution for those familiar with json.RawMessage.

On the other hand, goccy/go-yaml has the UseJSONUnmarshaler ( and UseJSONMarshaler ) option, which enables the use of UnmarshalJSON, allowing direct usage of json.RawMessage. Therefore, it would be beneficial to test the behavior when using json.RawMessage with this option enabled, as well as the behavior when using yaml.RawMessage.

Please add the test case the following.

  • UseJSONUnmarshaler and json.RawMessage
  • UseJSONUnmarshaler and yaml.RawMessage
  • UseJSONMarshaler and json.RawMessage
  • UseJSONMarshaler and yaml.RawMessage

@goccy
Copy link
Owner

goccy commented Mar 16, 2025

@cpuguy83 Do you have time to respond to the point I reviewed ? If not, I will close this PR and support this myself.

@goccy goccy added the waiting owner is waiting for a response label Mar 16, 2025
@goccy
Copy link
Owner

goccy commented Mar 26, 2025

@cpuguy83
I will wait until next week, and if there is no response, I will close this PR and handle this feature myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting owner is waiting for a response
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle json.RawMessage or introduce a similar type
5 participants