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

[PLATFORM-861]: Fix borked enum variant name formatting #61

Conversation

WilliamVenner
Copy link
Contributor

https://prima-assicurazioni-spa.myjetbrains.com/youtrack/issue/PLATFORM-861

Fixes a weird edge case where enum variant names were not being formatted correctly.

Added tests to ensure that we don't regress this in future.

@WilliamVenner WilliamVenner requested a review from a team as a code owner December 15, 2022 13:58
src/private.rs Outdated
/// Whether the type we're redacting is an Option<T> or not. Poor man's specialization! This is detected
/// by the proc macro reading the path to the type, so it's not perfect.
///
/// This could be improved & rid of in a number of different ways in the future:
///
/// * Once specialization is stabilized, we can use a trait to override redacting behaviour for some types,
/// * Once specialization is stabilized, we can use a trait to override redacting behavior for some types,
Copy link
Member

Choose a reason for hiding this comment

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

I can't believe a british dev would do this. Did it hurt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

💀

src/private.rs Outdated Show resolved Hide resolved
src/private.rs Outdated Show resolved Hide resolved
src/private.rs Outdated Show resolved Hide resolved
src/private.rs Outdated Show resolved Hide resolved
veil-macros/src/flags.rs Outdated Show resolved Hide resolved
veil-macros/src/lib.rs Outdated Show resolved Hide resolved
veil-macros/src/lib.rs Outdated Show resolved Hide resolved
@MaeIsBad
Copy link
Member

Moved my review over to #60 where it belongs

@WilliamVenner WilliamVenner force-pushed the PLATFORM-861/user-story/fix-borked-enum-variant-name-formatting branch from 15af92f to caba792 Compare December 16, 2022 15:29
@@ -168,7 +168,7 @@ pub(super) fn derive_redact(
for (variant, flags) in e.variants.iter().zip(variant_flags.into_iter()) {
// Variant name redacting
let variant_name = variant.ident.to_string();
let variant_name = if let Some(flags) = &flags.variant_flags {
let variant_name = if let Some(flags @ FieldFlags { skip: false, .. }) = &flags.variant_flags {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is for me: wth is @?

Copy link
Member

@MaeIsBad MaeIsBad Dec 19, 2022

Choose a reason for hiding this comment

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

it binds the flags variable and matches the pattern
https://doc.rust-lang.org/stable/rust-by-example/flow_control/match/binding.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

such an awesome feature :)

@WilliamVenner WilliamVenner merged commit 368216d into master Dec 19, 2022
@WilliamVenner WilliamVenner deleted the PLATFORM-861/user-story/fix-borked-enum-variant-name-formatting branch December 19, 2022 09:12
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

Successfully merging this pull request may close these issues.

3 participants