Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Valuable: Enum Representations #3051

Open
fosskers opened this issue Aug 8, 2024 · 0 comments
Open

Valuable: Enum Representations #3051

fosskers opened this issue Aug 8, 2024 · 0 comments

Comments

@fosskers
Copy link

fosskers commented Aug 8, 2024

Feature Request

Crates

tracing-subscriber (I think)

Motivation

Currently, plain variants of enums like:

enum Colour
  Red,
  Green,
  Blue,
}

serialise with the json / valuable features like so:

{"Red": []}

as a full JS object with an empty list, presumably to accommodate potential inner fields for variants that have them. Upstream logging / monitoring platforms have been observed to not play well with this representation, rendering it strangely or deleting it entirely as "empty data".

Serde supports multiple Enum Representations. Indeed, the default is to just render the variant name as-is, as can be seen here:

#[derive(Debug, serde::Serialize)]
enum Colour {
    Red,
    Green,
    Blue,
}

fn main() {
    let c = Colour::Red;
    let json = serde_json::to_string(&c).unwrap();
    println!("{json}");
}

Red

Proposal

It depends on how this representation is viewed. If the current representation is deemed a bug, then the default output could simply be altered in the case of variants that contain no inner fields.

On the other hand, in the spirit of tokio-rs/valuable#75 , this could be considered an opportunity to provide greater control over how enums are represented as JSON.

In the short term, my person vote is for the former, but would like to see the latter implemented eventually as well.

Alternatives

Changing the default representation for the fieldless case may surprise current users who currently depend on it. There's no way to know if anybody currently does, however.

Another alternative would be to return to something that was explicitly avoided in the original announcement, namely general logging support for anything that has serde::Serialize .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant