prost
prost copied to clipboard
Make a way so that prost-derive doesn't necessarily derive Debug for you
Sometimes you have structs that you want to give a custom implementation of Debug, but if you do that then they cannot derive Message which is unfortunate.
Would you take a patch that allows something like the following?
#[derive(Eq, PartialEq, Hash, Message)]
#[prost(NoDebug)]
pub struct MyStruct { ... };
impl core::fmt::Debug for MyStruct {
...
}
Alternatively, I think maybe we can have Message: Debug
, or maybe not have that requirement at all?
Either way this has my vote! It would be super helpful to not be forced to use the default derive(Debug)
since that limits you to only structs whose members all implement Debug
, and as @garbageslam mentioned it makes it impossible to use Prost with structs that implements a custom Debug
.
Yeah I think ideally we would not include the Debug
derive for #[derive(Message)]
, and instead have something like #[derive(prost::Debug)]
to add it, and have the code generator start outputting that. Another reason to do that is people get confused and think there isn't a Debug
impl for prost generated types.
im interested in this as well. removing Debug from Message: Debug + Send + Sync
can the code a lot smaller for embedded use (where i dont want debug or fmt at all in certain places). Dont know if the default should be to generate Debug or not. generating the impl by default matches the current behaviour, at least. i can give it a shot, unless @garbageslam is already on it.
i haven't done anything on this, feel free to give it a shot :smile:
cool. i'll fix this then. do we want it via codegen option in prost build so it can be set via prost_build::Config
or as a feature in prost-derive?
I would suggest that, it doesn't make sense for it to be an option in prost_build::Config
, because prost_build
is only related to the case that prost is making struct definitions from proto files. It merely uses the proc-macro offered by prost-derive.
Instead, this feature should be part of prost-derive config (affecting the code created by the proc-macro), and either do it like (1) make a proc macro attribute that affects derive(Message)
and enables the derivation of Debug
of as well, so like
#[derive(Message)]
#[prost(Debug)]
struct Foo {
#[prost(fixed64, tag = 1)]
a: i64,
#[prost(fixed64, tag = 2)]
b: i64,
}
To do this, you would look in the DeriveInput
object inside the prost-derive crate, and try to find the prost(Debug)
attribute among the attributes. See syn crate documentation here: https://docs.rs/syn/1.0.40/syn/struct.DeriveInput.html
OR do exactly what @danburkert suggested above, and just have a completely separate derive macro for the prost::Debug thing.
#[derive(Message)]
#[derive(prost::Debug)]
struct Foo {
#[prost(fixed64, tag = 1)]
a: i64,
#[prost(fixed64, tag = 2)]
b: i64,
}
Then you would have basically two proc-macros being offered by prost-derive. That's probably simpler unless for some reason rustc won't let you make a custom derive for Debug
like this (I don't know, I never tried)
I mainly meant the ergonomics of it for end users. if i wanna setup my codegen in my build.rs as is usually the case, i wanna be able to specify whether or not to derive debugs, so i still think prost-build needs to add that option in config.
as for implementation, the problem with prosts' Debug is that its tied internally to prosts' Field
type, as each field has a debug
method that does some wrapping, etc. Currently the building of debug is bundled with the derive for Message
, where it reuses that data it generates for message for debug. Separating it out will cause a bit of code duplication as i'd still need to generate Fields
again.
i think the easiest solution is actually what you mentioned first, as in, add an attribute and check it for the Derive input. that'd allow me to not change the code in prost-derive except for that check. i'll go with that, make a PR, and we can discuss it then :)
Big +1 to this - just ran into this being a problem where I'd like to customize my Debug
impls to provide better output to the tracing
crate and without the ability to override them you end up needing to do some ugly workarounds like making your own versions of Debug
and implementing it for a bunch of stuff.
Also needing this except for Default, as I'd like to set the default values myself. It would be great.
Instead of creating new configuration options for each derive argument, Config.exclude_type_attribute
would be even better and very close to the existing Config.type_attribute
@cortopy one issue is, in proto3 they dropped support for custom default fields, so if prost allows to have custom defaults it's somewhat a hazard for proto3 users. But maybe it's okay and helpful to proto2 users?
@garbageslam totally right and that's why sometimes I need to implement Default trait myself. I didn't mean the implementation of default
values but the trait itself. All types seem to get the Default trait derived and AFAIK there's no way to disable that
+1 on this, and any status update on this? it looks like there was a PR open from 2+ years ago? sometimes if I want to pretty print something, default implementation of Debug makes it bit hard, especially for nested messages.