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

Allow from_str to take Deserialize instead of DeserializeOwned #651

Closed
wants to merge 1 commit into from

Conversation

youknowone
Copy link

This change is introduced from here: https://github.com/toml-rs/toml/pull/457/files#diff-9dfce2824f02ba265b01f477a8d9821c7509063a170db31f465e3b176bbdab3dR40

Because DeserializeOwned allows fewer types than Deserialize, it introduces regression.
By looking serde documentation, Deserialize looking fits more for this function

@epage
Copy link
Member

epage commented Nov 13, 2023

Quoting from #505

From #505 (comment)

Second, we are shifting a compile time error into a runtime error for the easy path while, as you pointed out, a workaround is available for those that want to borrow. In API design, we should be emphasizing the safe path with the easy path while ideally still supporting the unsafe route. The current solutions fits that to a T. I don't know why the serde docs make the recommendation they do but in this case, I see it being useful.
I'm going to go ahead and close this and ask any further discussion to happen in the appropriate issue (#490).

@epage epage closed this Nov 13, 2023
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.

None yet

2 participants