-
Notifications
You must be signed in to change notification settings - Fork 30
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
Add index getter to Variant
#110
Conversation
/// Returns the index of the variant, if specified. | ||
pub fn index(&self) -> Option<u8> { | ||
self.index | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference here from the discriminant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The discriminant is only valid for "C Like" enums.
If both are specified then the index
takes precedence for encoding. We still retain the discriminant though for e.g. reconstructing the original type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay I see. Although we should keep in mind that there is a Rust RFC to allow for discriminants for data variants. Although then #[scale(index = N)]
would still override those which should be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that would still hold true. You are correct that index
specifically refers to the value from the attribute. And it can be different from the discriminator, but will take precedence for encoding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep in mind that there is a Rust RFC
If/when that lands and is stabilised we could deprecate the #[scale(index = N)]
attribute and only use discriminants. That would be nice (and a breaking change both to us and parity-scale-codec
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discriminant removed in #112
@@ -230,7 +230,7 @@ where | |||
&self.fields | |||
} | |||
|
|||
/// Returns the index of the variant. | |||
/// Returns the index of the variant, if specified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
No description provided.