althonos / packageurl.rs

Rust implementation of the Package URL specification.
MIT License
8 stars 6 forks source link

Consider splitting `PackageUrl` type into owned and borrowed variants? #9

Open alilleybrinker opened 1 year ago

alilleybrinker commented 1 year ago

Right now the PackageUrl type unifies owned and borrowed variants with Cow under the hood. While this works, it means that code wanting to work with owned PackageUrls either needs to thread lifetimes through their usage code which are largely irrelevant, or force 'static to ensure only string slices or non-borrowed underlying data are used.

An alternative would be to split between the owned and borrowed variant, with getter methods on the borrowed variant (and accessible to the owned variant via Deref impl), and with ownership conversion traits implemented as appropriate.

This would more closely match the String/str, PathBuf/Path, OsString/OsStr, CString/CStr split seen in other "stringy" types in the Rust standard library.

althonos commented 1 year ago

I think actually this crate needs a rewrite in terms of API, and I can see PackageUrl being an owned variant while a new PackageUrlBuilder can be used with owned or borrowed strings in order to build a package URL.

alilleybrinker commented 1 year ago

Oh that's interesting. Perhaps PackageUrl and PackageUrlRef, with both being able to be built by PackageUrlBuilder based on the use of owned or borrowed types? I'd be happy to talk through that API in more detail if you'd like!