cberner / redb

An embedded key-value database in pure Rust
https://www.redb.org
Apache License 2.0
3.07k stars 137 forks source link

Consider implementing `Value` for `OsString` and `PathBuf` #796

Closed CosmicHorrorDev closed 3 months ago

CosmicHorrorDev commented 3 months ago

&OsStr has a direct to/from bytes conversion for unix and wasi through std::os::{unix,wasi}::ffi::OsStrExt

Unfortunately -- unless I'm missing something -- there's no safe API to convert bytes to an &OsStr on windows, so instead this limits us to using an OsString with std::os::windows::ffi::OsStringExt which does provide a means of converting from bytes -> &[u16] -> OsString (and there's OsStrExt` for going the other direction)

With OsString out of the way you can also provide an implementation for PathBuf as well

cberner commented 3 months ago

I don't think this is a good idea. The encode function says: The byte encoding is an unspecified, platform-specific, self-synchronizing superset of UTF-8. By being a self-synchronizing superset of UTF-8, this encoding is also a superset of 7-bit ASCII.

And redb is designed to be portable by default. I would suggest storing a &[u8] or String in redb, or using your own wrapper type around OsString

CosmicHorrorDev commented 3 months ago

Ah it makes a lot of sense that this would break portability. I agree with all of your reasoning 👍