Add optional support for serde serialization & de-serialization

[Lepidopteran]

Hello!

When using lofty in my project that uses axum, I noticed I couldn't serialize TagType, which isn't a big deal as I can just recreate the enum with serde::Serialize and serde::Deserialize attached to it.

However it would nice to have optional support for serde via an optional feature.

A few examples that would be nice to able serialize would be TagType, PictureType, and PictureInformation

I could definitely create a pull request, if you'd like.

[uklotzde]

I don't think that this would be suitable. As long as there is no canonical, serialized representation for those types it should not be implemented in lofty.

[Lepidopteran]

I don't believe I fully understand why this wouldn't be suitable.

Wouldn't adding serde as an optional dependency like this:

[features]
#...
serde = ["dep:serde"]

[dependencies.serde]
version = "1.0"
features = ["derive"]
optional = true

Then derive serde::Serialize & serde::DeSerialize to the types that can automatically be serialized by serde if the feature is enabled be sufficient?

Example adding optional serde support for TagType:

 /// The tag's format
+ #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
#[non_exhaustive]
pub enum TagType {
// ...
}

If I'm misunderstanding something, please let me know.

Again if this feature can/should not be added, it's not a huge deal, all I have to do is create duplicate types in my project that derives from serde::Serialize and serde::Deserialize like so:

/// Duplicate of [`TagType`](lofty::tag::TagType) from [lofty](https://crates.io/crates/lofty), except with [`Serialize`](serde::Serialize) and [`Deserialize`](serde::Deserialize) traits implemented.
#[non_exhaustive]
#[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
pub enum TagType {
    Ape,
    Id3v1,
    Id3v2,
    Mp4Ilst,
    VorbisComments,
    RiffInfo,
    AiffText,
}

impl From<TagType> for lofty::tag::TagType {
    fn from(value: TagType) -> Self {
        match value {
            TagType::Ape => lofty::tag::TagType::Ape,
            TagType::Id3v1 => lofty::tag::TagType::Id3v1,
            TagType::Id3v2 => lofty::tag::TagType::Id3v2,
            TagType::Mp4Ilst => lofty::tag::TagType::Mp4Ilst,
            TagType::VorbisComments => lofty::tag::TagType::VorbisComments,
            TagType::RiffInfo => lofty::tag::TagType::RiffInfo,
            TagType::AiffText => lofty::tag::TagType::AiffText,
        }
    }
}

impl From<lofty::tag::TagType> for TagType {
    fn from(value: lofty::tag::TagType) -> Self {
        match value {
            lofty::tag::TagType::Ape => TagType::Ape,
            lofty::tag::TagType::Id3v1 => TagType::Id3v1,
            lofty::tag::TagType::Id3v2 => TagType::Id3v2,
            lofty::tag::TagType::Mp4Ilst => TagType::Mp4Ilst,
            lofty::tag::TagType::VorbisComments => TagType::VorbisComments,
            lofty::tag::TagType::RiffInfo => TagType::RiffInfo,
            lofty::tag::TagType::AiffText => TagType::AiffText,

            _ => TagType::Id3v2,
        }
    }
}

[Serial-ATA]

I could see the value in having the basic *Type enums (FileType, TagType, PictureType, MimeType) support serialization for use in a database. They'd all just be basic strings anyway. I'd avoid something like ItemKey, though.

Feel free to make a PR if you want :)

[Lepidopteran]

Will Do! Would you like to me to exempt PictureInformation and just do the *Type enums?

[uklotzde]

I don't believe I fully understand why this wouldn't be suitable.

Not everything that is technically possible or easy is sensible.

IMHO it is out of scope of this library and would add an unnecessary maintenance burden.

Even for simple strings you would need to make an opinionated decision which formatting to use (camelCase, PascalCase, ...). This might be inconsistent with how enumerations are supposed to be serialized in the outer/enclosing context. It is unlikely that the lofty types are used in isolation.

In non-trivial projects you should always keep the serialization or schema separate from the Domain Model. APIs and persistence have different requirements for serialization. See also: Hexagonal or Clean Architecture. Sticking serde attributes to every type is not recommended.

Use a simple newtype for each use case and tune the serde attributes according to your needs. Like you did in your example. Tedious but worth it.

Valid exception: Date/time libraries that use standardized string formats (ISO/RFC) for serialization and offer support for serde as an optional feature.

[Lepidopteran]

I've created a pull request implementing basic serde support for *Type enums.

If it doesn't get merged, it is not a big issue. I can continue creating duplicate types like I've been doing.

@uklotzde brings some valid points about separation of concerns and about formatting (though I do believe serde just parses from the name and does not transform the string by default).

[Serial-ATA]

Even for simple strings you would need to make an opinionated decision which formatting to use (camelCase, PascalCase, ...)

Casing should only ever matter for keys, not values. At least as far as I can imagine.

Looks like image also offers serde support for simple enums:

https://docs.rs/image/latest/src/image/image.rs.html#26