♻️ Prep for spec string changes

[CosmicHorrorDev]

Related #11

Some prep work before getting more of the string types to be spec compliant

Pending Q: More generics to get rid of all of the .into()s? Maybe some From<&str> and From<String> for Value and some generics on .insert()?

[zeenix]

Some prep work before getting more of the string types to be spec compliant

Nice! I knew that we'll need to move to Cow for escaping strings at least.

Pending Q: More generics to get rid of all of the .into()s? Maybe some From<&str> and From<String> for Value and some generics on .insert()?

Sure, I don't really mind the .into()s either. Our code is still simple enough with those so up to you. If you add those, it'd make sense to add them for all the variants though.

[CosmicHorrorDev]

Sure, I don't really mind the .into()s either. Our code is still simple enough with those so up to you. If you add those, it'd make sense to add them for all the variants though.

I'll work through that in a follow-up PR, so that I can make all of the changes at the same time then 👍

[zeenix]

I noticed this doesn't change the strings in the cargo module. Is that intentional?

[CosmicHorrorDev]

Ah I somehow missed those. I'll go through and change those too in some follow-up work

---

Aside: long-term I think that it would make sense for the cargo stuff to be in its own crate that depends on tomling (cargo-tomling 🤔), since it's a pretty specific domain that's much more likely to have API changes in the future compared to a toml parser

[zeenix]

Ah I somehow missed those. I'll go through and change those too in some follow-up work

👍

Aside: long-term I think that it would make sense for the cargo stuff to be in its own crate that depends on tomling (cargo-tomling 🤔), since it's a pretty specific domain that's much more likely to have API changes in the future compared to a toml parser

That makes a lot of sense, yes. However, I mainly created this crate for cargo manifest parsing and to reduce the number of dependencies in proc-macro-crate (see bkchr/proc-macro-crate#37) so I'd prefer keeping it in the same crate.[^1]

Having said that, I'd consider making cargo-toml feature a non-default feature.

since it's a pretty specific domain that's much more likely to have API changes in the future compared to a toml parser

We can make use of the fact that Cargo manifest itself doesn't usually have breaking changes (does it at all? 🤔) and make the API as thought-through as possible. I can understand if you don't have interest in that part so I can take care of that. The next release will have to be a breaking change though but I'm hoping there won't be many. Fortunately this crate is still in its infancy and hasn't seen an adoption yet so we breaking changes aren't that big a deal right now (we'll be bumping the semver each time still of course).

---

^1: Personally speaking, I prefer a big number of tiny crates as that makes things so much more modular and reusable but a lot of folks unfortunately don't and I already get a lot of hate for number of dependencies of zbus (the reason I got interested in reducing dependencies of proc-macro-crate and in turn creation of this crate).

[CosmicHorrorDev]

Unless I'm missing something I think that the cargo-toml parts of this crate are pretty overkill for what proc-macro-crate requires. You could get by easily enough with the raw parsing API and avoid having to pull in serde at all

[zeenix]

Unless I'm missing something I think that the cargo-toml parts of this crate are pretty overkill for what proc-macro-crate requires. You could get by easily enough with the raw parsing API and avoid having to pull in serde at all

That's a very good point. I guess given that we don't really have many users (if any) yet, we can just ditch the cargo-toml stuff completely. I can always resurrect the code in a new crate if people ask for it. Could you provide a PR that does that? Note that the README will need to be updated too.