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

proposal: encoding: make stdlib types implement TextMarshaler/BinaryMarshaler consistently #10275

Open
lavalamp opened this issue Mar 27, 2015 · 7 comments

Comments

@lavalamp
Copy link

@lavalamp lavalamp commented Mar 27, 2015

For example, the standard library has a decent encoding/json package, and a great time.Duration object. But these objects don't play well together; if you want to encode a Duration in a json blob, you'll have to make a custom type that calls .String()/ParseDuration() when Get/SetJSON are called. It would be nice if they worked together by default.

...Unfortunately, the cross product of all the useful encoding packages with all the useful types in the standard library is probably quite large, so I'll understand if you say "no" to that. Perhaps at least time.Time and time.Duration could be supported?

@mikioh mikioh changed the title Feature request: std lib: common types to support common encodings all: common types to support common encodings Mar 27, 2015
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Apr 10, 2015

This isn't possible to change now, as it would change the encodings produced by programs that exist today.

@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc added the Go2 label Apr 10, 2015
@lavalamp

This comment has been minimized.

Copy link
Author

@lavalamp lavalamp commented Apr 20, 2015

@rsc Let me push back just a little on that, I don't see why it's necessarily true.

For example, again taking encoding/json with time.Time & Duration: I don't see a reason why the time.SetJSON method couldn't accept as input the string that's currently emitted.

I can see why you'd want to change the output of time.Duration, but I'm not sure why it'd be a problem to make the SetJSON method accept both the new output format (e.g., "5s") and the old (which is nanoseconds, without quotes). Those cases are super easy to distinguish.

...I agree the generalized cross product of types w/ encodings is probably going to have many much more difficult cases.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Jun 16, 2017

Replying to a comment from 2015, but we can't change the encoding outputs now because then if a new version of Go is sending data to an older version, the older version will not understand it.

@rsc rsc changed the title all: common types to support common encodings encoding: make stdlib types implement TextMarshaler/BinaryMarshaler consistently Jun 16, 2017
@lavalamp

This comment has been minimized.

Copy link
Author

@lavalamp lavalamp commented Jun 16, 2017

That means it'd have to be opt in, e.g. with paired type TextMarshaller2 interface { ... V2() } and json.NewEncoder().UseV2().Encode(). Obviously that's hideous and I don't think it should be done, but it is possible.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Jun 17, 2017

It means it would have to wait for Go 2.

@rsc rsc changed the title encoding: make stdlib types implement TextMarshaler/BinaryMarshaler consistently proposal: encoding: make stdlib types implement TextMarshaler/BinaryMarshaler consistently Jun 17, 2017
@gopherbot gopherbot added the Proposal label Jun 17, 2017
@ianlancetaylor ianlancetaylor modified the milestones: Unplanned, Proposal Jan 9, 2018
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 9, 2018

If we change this in Go 2--if we add marshaling methods for standard library types--we will have to provide some mechanism for Go 2 programs to read Go 1 data. One approach would be to only add marshaling methods in new versions of the packages, and to permit the old and new versions of the package to both be imported in the same program.

@aviau

This comment has been minimized.

Copy link

@aviau aviau commented Jun 7, 2018

we can't change the encoding outputs now

@rsc Would you consider implementing Unmarshal interfaces?

We can do this right now, can't we?

#25705 Was closed because apparently implementing UnmarshalText in net/url would break existing code. Maybe I am missing it, so please correct me if I am wrong, but I don't see how that is true.

There are plenty of use cases for code that parses URLs and this seems like an easy fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.