typst / ecow

Compact, clone-on-write vector and string.
Apache License 2.0
205 stars 16 forks source link

Add `From<&EcoString> for EcoString` impl #41

Closed idlercloud closed 4 days ago

idlercloud commented 1 week ago

Add an impl like impl From<&String> for String to allow convert &EcoString to EcoString. In my use case, this will enable function to take param as impl Into<EcoString>, so it can receive either &EcoString or EcoString.

laurmaedje commented 1 week ago

I'm not a huge fan of this as it could hide unnecessary clones where a move would be possible. But I guess consistency with std is more important.

idlercloud commented 1 week ago

Sorry for accidentally committing irrelevant code. I have revert it.

Since std already did this, it looks ok for us to do the same.

However, I briefly investigated other string optimization crates in the ecosystem, and it seems that none of them implement this.

Considering that cloning EcoString is cheap, for my use case, simply passing an EcoString seems to be acceptable.

So feel free to merge or reject.

laurmaedje commented 4 days ago

Let's follow std here rather than other third party crates.

I got to fixing CI so that we don't get a red checkmark on main after merge.

Thanks!