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

Decode: assign empty struct to empty defined sections #879

Merged
merged 2 commits into from Jul 12, 2023

Conversation

dbarrosop
Copy link
Contributor

@dbarrosop dbarrosop commented Jun 3, 2023

Fixes #878

Not sure what the gate in the patched method was intended to do but it was preventing from creating pointers to structs correctly when a section was defined. This change ensures that original_document == Marshal(Unmarsal(original_document) which isn't the case without this patch.

@pelletier pelletier added the bug Issues describing a bug in go-toml. label Jun 6, 2023
@pelletier
Copy link
Owner

Thanks a lot for the patch, and sorry for the delay! Not sure what that check was about either, but I think having round-trip equality is a good property to have.

@pelletier pelletier changed the title if a pointer to a struct is defined assign empty struct correctly Decode: assigned empty struct to empty defined sections Jun 6, 2023
@pelletier
Copy link
Owner

Looks like removing this check either made some code unused, or created untested cases. Do you mind taking a look and removing the code or adding relevant tests so that we are sure we don't miss anything? Otherwise, I can check next weekend.

@dbarrosop
Copy link
Contributor Author

Sorry for the delay, I was travelling during the week. I tried looking into this but I am afraid I am not familiar enough with the codebase to understand why the coverage went down or how to increase it back. If this is something you could help with I'd really appreciate it. Thanks!

@dbarrosop
Copy link
Contributor Author

@pelletier do you think you could help with the coverage, please?

@pelletier
Copy link
Owner

Done; thank you for the nudge!

@pelletier pelletier merged commit e183db7 into pelletier:v2 Jul 12, 2023
9 checks passed
@dbarrosop
Copy link
Contributor Author

Awesome! Thanks for the help :)

@dbarrosop dbarrosop deleted the nil branch July 13, 2023 07:16
@pelletier pelletier changed the title Decode: assigned empty struct to empty defined sections Decode: assign empty struct to empty defined sections Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues describing a bug in go-toml.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

omitempty returns nil for defined empty sections
2 participants