Remove ItemKey::Unknown
#521
Comments
|
Hi! I understand the motivation behind simplifying the ItemKey enum and removing ItemKey::Unknown, but I'd like to offer a user perspective that may have been overlooked. I'm currently building a music library and metadata API where users can scan, view, and edit tags generically across different formats (MP3, FLAC, M4A, etc.). For this use case, ItemKey::Unknown has been extremely valuable. It allows me to support custom or non-standard tags like RATING, MOOD, BPM_LABEL, and others without needing to write backend-specific logic for each format. While these tags may seem niche from a spec point of view, they are actually very common in real-world scenarios — especially for advanced playback systems, smart playlisting, DJ software, and even consumer apps that want to store user preferences. By removing ItemKey::Unknown, I would be forced to detect the tag backend (e.g., Vorbis, ID3v2, MP4) and write logic per format just to set or read a single custom field — which breaks the abstraction that Tag provides. For example, I’ve encountered many high-quality FLAC files in the wild where fields like metadata.set_total_tracks(tag.track_total().map(|n| n as u16).or_else(|| {
tag.get_string(&ItemKey::Unknown("TRACKTOTAL".to_string()))
.and_then(|s| s.trim().parse::<u16>().ok())
}));
metadata.set_total_discs(tag.disk_total().map(|n| n as u16).or_else(|| {
tag.get_string(&ItemKey::Unknown("DISCTOTAL".to_string()))
.and_then(|s| s.trim().parse::<u16>().ok())
}));I completely understand if ItemKey::Unknown needs to be deprecated or discouraged for performance or consistency reasons. But completely removing it would break a lot of real, practical use cases that rely on flexible metadata editing. Thanks for your work on Lofty — it’s a great library and I really appreciate the attention to correctness and long-term design! |
|
@Undead34 Thanks for sharing your thoughts!
I'm always open to adding more And for the record, lofty-rs/lofty/src/tag/item.rs Lines 337 to 413 in d9eb83b
You should be able to do: metadata.set_total_tracks(tag.track_total().map(|n| n as u16).or_else(|| {
tag.get_string(&ItemKey::TrackTotal)
.and_then(|s| s.trim().parse::<u16>().ok())
}));
metadata.set_total_discs(tag.disk_total().map(|n| n as u16).or_else(|| {
tag.get_string(&ItemKey::DiscTotal)
.and_then(|s| s.trim().parse::<u16>().ok())
}));
lofty-rs/lofty/src/tag/item.rs Lines 371 to 373 in d9eb83b Does that not work? |
|
Thanks for the clarification and for maintaining such a great library! 🙏 I just tested using When I have a free moment, I’d be happy to contribute a small docs PR with these examples to help others avoid the same confusion. Thanks again! 😊 |
ItemKey::Unknownhas served its purpose, and has had better alternatives for a couple years now. It's been a source of confusion numerous times, and its existence also requires extra verification logic on conversions. I'd like to see it gone.Comment taken from #520
ItemKey::Unknownwas kind of a hacky way to retain tags that couldn't be mapped to concreteItemKeys. There have been alternatives for awhile now though, with SplitTag, MergeTag, andGlobalOptions::preserve_format_specific_items()(see #302 for more info on that).I envision people taking their
Tags, converting them to a concrete format (e.g.VorbisComments), and then adding their custom fields to that. I'd rather not have a way to generically add custom fields, since the mappings for the concreteItemKeyvariants are known to be valid, but withUnknown, there's extra verification that needs to be done on conversions that could be skipped if I just get rid of it entirely.You'd go from:
to:
ItemKeysupports the keys that the vast majority of users will need, anything else would be niche and require a little extra effort working with the concrete formats.The text was updated successfully, but these errors were encountered: