-
Notifications
You must be signed in to change notification settings - Fork 366
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
Replace MarkerShape
with code-generated enum
type
#5336
Conversation
@@ -10,13 +10,16 @@ namespace rerun.blueprint.archetypes; | |||
/// The top-level description of the Viewport. | |||
table ContainerBlueprint ( | |||
"attr.rerun.scope": "blueprint", | |||
"attr.rust.derive": "Default", |
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 old code had a bug, which defaulted the container_kind
to the invalid value 0
You shouldn't need an extra attribute to find the associated enum type.
E.g.: diff --git a/crates/re_types_builder/src/objects.rs b/crates/re_types_builder/src/objects.rs
index 07d1f0fb38..e1d32f5516 100644
--- a/crates/re_types_builder/src/objects.rs
+++ b/crates/re_types_builder/src/objects.rs
@@ -795,6 +795,7 @@ impl ObjectField {
let attrs = Attributes::from_raw_attrs(field.attributes());
+ dbg!(enums[field.type_().index() as usize]);
let typ = if let Some(enum_type) = attrs.try_get(&fqname, crate::ATTR_ENUM_TYPE) {
// Hack needed because flattbuffers report fields of enum types as integers.
Type::Object(enum_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.
Looks great except for that superfluous enum_type
attribute and the miscellaneous hacks that come with it.
"attr.rust.derive": "PartialEq, Eq, PartialOrd, Copy", | ||
"attr.rust.repr": "transparent" | ||
enum MarkerShape: byte ( | ||
"attr.docs.unreleased" |
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.
why unreleased?
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 generated doclink changes from https://docs.rs/re_types/latest/re_types/components/struct.MarkerShape.html to https://docs.rs/re_types/latest/re_types/components/enum.MarkerShape.html which doesn't exist (yet)
What
enum
in codegen to specify constant-constants #3384Also replaces
Corner2D
andContainerKind
, but those are blueprint only and thus not user facing (yet).The user-facing code should stay mostly the same.
I also improved the generated documentation for all components and datatypes
Checklist
main
build: app.rerun.ionightly
build: app.rerun.io