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

impl rename_all for #[derive(Display)] (#216) #443

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ModProg
Copy link
Contributor

@ModProg ModProg commented Feb 2, 2025

Resolves #216

Checklist

  • Documentation is updated (if required)
  • Tests are added/updated (if required)
  • CHANGELOG entry is added (if required)

@ModProg ModProg force-pushed the display-rename-all branch 2 times, most recently from 23cd199 to d76973d Compare February 2, 2025 23:40
@ModProg ModProg force-pushed the display-rename-all branch from d76973d to 2ebbf3e Compare February 2, 2025 23:43
@tyranron tyranron added this to the 2.1.0 milestone Feb 17, 2025
/// Interpolation [`FmtAttribute`].
fmt: Option<FmtAttribute>,

/// Addition trait bounds.
bounds: BoundsAttribute,

/// Rename unit enum variants following a similar behavior as [`serde`](https://serde.rs/container-attrs.html#rename_all).
rename_all: Option<T>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ModProg instead of modifying the ContainerAttributes with an additional slot (which is useless for the Debug), I think it's better to leave it "as is", and:

  1. Move RenameAllAttribute into display module.
  2. Inside display module do:
    struct ContainerAttributes {
        rename_all: RenameAllAttribute,
        common: super::ContainerAttributes,
    }
    and parse it as Either<RenameAllAttribute, super::ContainerAttributes> to omit repeating parsing of the super::ContainerAttributes twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought It'd make sense to allow to have rename_all and bounds in the same attribute. But that will only at most save one line anyway.

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

Successfully merging this pull request may close these issues.

Serde like rename for Display trait on enum
3 participants