feat(toml_edit): Implement IntoDeserializer for Item

[sxyazi]

I'm working on a config parser based on toml_edit that requires the implementation of IntoDeserializer for interoperability with serde.

I noticed that toml_edit::Item already has an into_deserializer() method, but it's not public. So, this PR exposes it by implementing the IntoDeserializer trait.

[epage]

From https://github.com/toml-rs/toml/blob/main/CONTRIBUTING.md

Already have an idea? It might be good to first create an issue to propose it so we can make sure we are aligned and lower the risk of having to re-work some of it and the discouragement that goes along with that.

For example, its not clear why you specifically want Item when we provide this for Document and Value. In particular, there are multiple expectations people might have for Item which is why we intentionally left it off.

[sxyazi]

Sorry, I should have created an issue first, or at least explained clearly what I wanted to do in the PR.

I want to parse the toml:

# ...

[editor]
mouse = true
theme = "onedark"

# ...

into a struct:

#[derive(serde::Deserialize)]
struct Editor {
  mouse: bool,
  theme: String,
}

Config parsing is part of the system, and elsewhere in the system there is already a toml_edit::Item corresponding to [editor], so I want to reuse it instead of encoding it as a string and then pass it to the toml crate.

[coveralls]

Pull Request Test Coverage Report for Build 14008023700

Details

• 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 67.853%

---

Totals
Change from base Build 13929193067:	0.0%
Covered Lines:	3738
Relevant Lines:	5509

---

💛 - Coveralls

[epage]

After refreshing myself on this code and the motivations for its design, my concerns are

• I am uncertain what the right line is for what deserializers to provide. Currently, we limit it to just the types that can be parsed (Deserializer, ValueDeserializer). Do we support a TableDeserializer, ArrayDeserializer, or KeyDeserializer?
• We shouldn't be saying that a ValueDeserializer is an Items deserializer. This would require a refactor commit that pulls out an ItemDeserializer (private) that ValueDeserializer wraps with a follow up commit that publicly exposes ItemDeserializer.

Also, is there a reason you are using toml_edit::Item rather than toml::Value?

[sxyazi]

Do we support a TableDeserializer, ArrayDeserializer, or KeyDeserializer?

In my case, I don't need them since I don't need to know the specific type of the content, and I just need to know it's an Item that can be deserialized into my struct T. Maybe for now, just keep them private as internal implementation details until someone needs them?

We shouldn't be saying that a ValueDeserializer is an Items deserializer.

Makes sense, I will try to refactor it - I will set the PR as draft until I complete the refactoring (expected in the next day or two).

is there a reason you are using toml_edit::Item rather than toml::Value?

toml::Value will lose span information, I need this information to provide more user-friendly errors, such as line numbers and surrounding content, when the user has configured an incorrect value, like configuring a number as a string.

[epage]

In my case, I don't need them since I don't need to know the specific type of the content, and I just need to know it's an Item that can be deserialized into my struct T. Maybe for now, just keep them private as internal implementation details until someone needs them?

I'm trying to look for the principles of what the API should be, not handle one off cases.

[sxyazi]

My knowledge of API design is very limited, so I might not be the right person to answer this.

Before finding such principles, is there any chance of accepting this one-off PR? If not, I can close it - or I can create a separate issue for discussion if that would help. It's all up to you :)

[sxyazi]

Hey @epage, I have separated ValueDeserializer into a new ItemDeserializer, and now ValueDeserializer is just a wrapper of ItemDeserializer.

Let me know if there are any other changes needed.

[sxyazi]

Perhaps it would be better to change crate::Item to crate::Value here, but I'm not sure if we should introduce more unrelated changes, as it might make the review more difficult.

If you prefer, I can make this change in a new commit or a separate refactor PR.