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

Top-level shorthand for ValueSerializer #565

Open
Nemo157 opened this issue May 31, 2023 · 3 comments
Open

Top-level shorthand for ValueSerializer #565

Nemo157 opened this issue May 31, 2023 · 3 comments
Labels
A-serde Area: Serde integration C-enhancement Category: Raise on the bar on expectations

Comments

@Nemo157
Copy link

Nemo157 commented May 31, 2023

In docs.rs we serialize Vec<String>s to pass through cargo's --config flag, this caused issues on updating from 0.5 to 0.7 as it started erroring. It looks like replacing this with ValueSerializer would take a fair chunk of code, I was wondering if you would consider having something like fn value_to_string<T: Serialize + ?Sized>(value: &T) -> Result<String, Error> as a top-level function (or maybe nested at value::to_string) wrapping this code.

@epage
Copy link
Member

epage commented May 31, 2023

It looks like its 1-5 lines of code, depending on the approach taken and where rustfmt line wraps. For how often this functionality is needed, I'm a bit unsure of the pay off of this. By "this" I also include things like having to come up with names (e.g. imo use is overused and people are like to use toml::value::to_string, losing critical context and possibly conflicting).

@epage epage added C-enhancement Category: Raise on the bar on expectations A-serde Area: Serde integration labels May 31, 2023
@Nemo157
Copy link
Author

Nemo157 commented May 31, 2023

Hmmm, I don't see how to get it down to 1 line, my expectation was 3

fn value_to_string<T: serde::Serialize + ?Sized>(value: &T) -> Result<String, Error> {
  let mut output = String::new();
  value.serialize(toml::ser::ValueSerializer::new(&mut s))?;
  output
}

(in cases where you have an existing string to write to and need peformance it'd only require the middle line, but for low-performance cases creating some temporary strings is more readable).

@epage
Copy link
Member

epage commented May 31, 2023

One line would be Value::try_from(value).to_string() which would have higher overhead (converts to an intermediate state).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-serde Area: Serde integration C-enhancement Category: Raise on the bar on expectations
Projects
None yet
Development

No branches or pull requests

2 participants