-
Notifications
You must be signed in to change notification settings - Fork 500
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
feat: Name
trait + Any
encoding support
#896
Changes from all commits
1d0f812
01fde2d
c41a155
46e73e0
cfe8a99
fe92a0f
b783481
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
//! Support for associating type name information with a [`Message`]. | ||
|
||
use crate::Message; | ||
use alloc::{format, string::String}; | ||
|
||
/// Associate a type name with a [`Message`] type. | ||
pub trait Name: Message { | ||
/// Type name for this [`Message`]. This is the camel case name, | ||
/// e.g. `TypeName`. | ||
const NAME: &'static str; | ||
|
||
/// Package name this message type is contained in. They are domain-like | ||
/// and delimited by `.`, e.g. `google.protobuf`. | ||
const PACKAGE: &'static str; | ||
|
||
/// Full name of this message type containing both the package name and | ||
/// type name, e.g. `google.protobuf.TypeName`. | ||
fn full_name() -> String { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should these be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If Otherwise it's not really possible to construct a |
||
format!("{}.{}", Self::NAME, Self::PACKAGE) | ||
} | ||
|
||
/// Type URL for this message, which by default is the full name with a | ||
/// leading slash, but may also include a leading domain name, e.g. | ||
/// `type.googleapis.com/google.profile.Person`. | ||
Comment on lines
+22
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps this could include some of the documentation I added to the internal |
||
fn type_url() -> String { | ||
format!("/{}", Self::full_name()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could potentially benefit from an easier way to customize the URL authority (i.e. hostname+port). It was a bit tricky to do this for the "well known" types: |
||
} | ||
} |
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.
Can this just live in
prost_types
, why does it need to live in the core crate?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.
I was hoping it could eventually be autogenerated by
prost-build
, at least optionally. We're hand annotating them right now and it's getting quite cumbersome.It seems like
prost-build
currently only refers to types/traits inprost
. If it were optional and it could refer to types/traits inprost-types
, I'd be fine with theName
trait living there.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.
I see your point, do we just want to go ahead and do the work to also add the prost-build parts? The next release will be breaking and I would be ok if we add something like that.