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

Small refactor in EnumGenerator.kt #2097

Closed

Conversation

david-perez
Copy link
Contributor

Some Rust code blocks have been trivially made more uninterrupted and
DRYer.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Some Rust code blocks have been trivially made more uninterrupted and
DRYer.
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link
Contributor

@jjant jjant left a comment

Choose a reason for hiding this comment

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

Nice!
I think we might be able to condense the block below as well.

Comment on lines 225 to 247
docs(
"""
Opaque struct used as inner data for the `Unknown` variant defined in enums in
the crate
the crate.

While this is not intended to be used directly, it is marked as `pub` because it is
part of the enums that are public interface.
""".trimIndent(),
""",
)
meta.render(this)
rust("struct $UnknownVariantValue(pub(crate) String);")
rustBlock("impl $UnknownVariantValue") {
// The generated as_str is not pub as we need to prevent users from calling it on this opaque struct.
rustBlock("pub(crate) fn as_str(&self) -> &str") {
rust("&self.0")
// The generated `as_str` is not `pub` as we need to prevent users from calling it on this opaque struct.
rust(
"""
struct $UnknownVariantValue(pub(crate) String);
impl $UnknownVariantValue {
pub(crate) fn as_str(&self) -> &str {
&self.0
}
}
}
""",
)
}
}
Copy link
Contributor

@jjant jjant Dec 13, 2022

Choose a reason for hiding this comment

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

Can this be written as this (the syntax might be slightly off)?

rustTemplate("""
                /// Opaque struct used as inner data for the `Unknown` variant defined in enums in
                /// the crate.
                /// While this is not intended to be used directly, it is marked as `pub` because it is
                /// part of the enums that are public interface.
                #{Metadata:W}
                struct $UnknownVariantValue(pub(crate) String);
                impl $UnknownVariantValue {
                    pub(crate) fn as_str(&self) -> &str {
                        &self.0
                    }
                }
""",
"Metadata" to meta::render
)

@@ -299,7 +309,7 @@ private fun RustWriter.renderForwardCompatibilityNote(
variant in a current version of SDK, your code should continue to work when you
upgrade SDK to a future version in which the enum does include a variant for that
feature.
""".trimIndent(),
""",
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting that trimIdent can be removed 🤔 I had to add it only because a pre-commit hook complained about it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think they can always be removed, and perhaps the pre-commit hook error message is not clear/just wrong.

@github-actions
Copy link

github-actions bot commented Jan 3, 2023

A new generated diff is ready to view.

A new doc preview is ready to view.

@david-perez
Copy link
Contributor Author

Closing because of staleness; these improvements were made in #2334.

@jjant jjant deleted the davidpz/small-refactor-in-_enumgenerator.kt_ branch March 13, 2023 17:53
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.

4 participants