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

better detection of pointer receivers #979

Open
wants to merge 2 commits into
base: v3
Choose a base branch
from

Conversation

aathan
Copy link

@aathan aathan commented Aug 9, 2023

I did not attempt to fully grok the yaml marshaler implementation but it seems to me to be broken, in the sense that it does not correctly pick up Marshaler implementations that take pointer receivers when a non-pointer struct is marshaled. While at some point the yaml marshaler may want to treat pointers differently from non-pointers (e.g., to create anchors and references), this is not currently a feature so I think "casting" everything to a pointer before doing the type assertion is a valid way to go. time.Time doesn't need a custom marshaler since that type already implements TextMarshaler; and there seemed to be inconsistent handling of Duration and Time pointers vs non-pointers. This is cleaned up in the PR because the "cast" to a pointer folds. I saw other PRs that introduce the ability to set Time formatters etc but I think maybe a more general impl such as passing in a func() that gets called on all types that don't have a Marshaler so as to be able to override the default behavior ...

@aathan
Copy link
Author

aathan commented Aug 9, 2023

See #981 for combined PR.

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