Support non-str values in a Rodeo #32
Conversation
| impl<'a, K, V: ?Sized> iter::FusedIterator for Strings<'a, K, V> {} | ||
|
|
||
| /// Trait for types that can be interned within a `Rodeo` | ||
| pub trait Internable: Hash + Eq + AsRef<Self> { |
There was a problem hiding this comment.
Intentionally public, though should maybe be an unsafe trait, as implementations are expected to round-trip correctly.
| } | ||
|
|
||
| fn as_bytes(&self) -> &[u8] { | ||
| self.to_str() |
There was a problem hiding this comment.
Rust intentionally doesn't allow you to get an OsStr as bytes cross-platform
| S: BuildHasher + Clone + Default, | ||
| { | ||
| #[cfg_attr(feature = "inline-more", inline)] | ||
| fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> | ||
| where | ||
| D: Deserializer<'de>, | ||
| { | ||
| let deser_map: HashMap<String, K> = HashMap::deserialize(deserializer)?; | ||
| let deser_map: HashMap<K, Vec<u8>> = HashMap::deserialize(deserializer)?; |
There was a problem hiding this comment.
Swapped because JSON requires hashmap keys be strings, and that would require escaping/unescaping complexity
There was a problem hiding this comment.
I'd rather let the deserializer manage things themselves, there's a lot more formats than json
There was a problem hiding this comment.
Fair enough, though also, at least with this ordering it's less likely to break a lot of use cases. The tests use JSON, so I figured keeping it working would be considered more important than simply swapping the position here. Would you rather I swapped it back anyways?
|
I did some work on this in my
In my branch I pretty much just give up on the first two problems, I use |
|
Do you know if this helps solve the problem with OsStr? |
|
FWIW here's this branch rebased on top of master: https://github.com/rlabrecque/lasso/tree/rlabrecque/internable Seems to work really well for my needs so far. |
Personally, I'd rather have an imperfect and unstable API for this now than have a perfect API in two years. This gives anybody who needs this functionality now something to at least work with, while making all of the downsides non-commital. (If |
Adds a new public trait,
Internable, which is by default implemented forstr,OsStr,Path, and[u8]. This is a noticeably breaking change, due to the addition of aVgeneric on most things, as well as regressions for a couple type-inference locations.I can add in #26 now if desired, as this seems a good time to make that change, if breaking inference anyways.
fixes #23